On 22.03.2018 23:53, Alvaro Herrera wrote:
> The whole IsProjectionFunctionalIndex looks kinda bogus/ugly to me. Set
> the boolean to false, but keep evaluating anyway? But then, I thought
> the idea was to do this based on the reloption, not by comparing the
> expression cost to a magical (unmodifiable) value?
The idea is very simple, I tried to explain it in the comments. Sorry if
my explanation was not so clear.
By default we assume all functional indexes as projectional. This is
done by first assignment is_projection = true.
Then we check cost of index function.
I agree that "magical (unmodifiable) value" used as cost threshold is
not a good idea. But I tried to avoid as much as possible adding yet
another configuration parameter.
I do no think that flexibility here is so important. I believe that in
99% cases functional indexes are projectional,
and in all other cases DBA cna explicitly specify it using
recheck_on_update index option.
Which should override cost threshold: if DBA thinks that recheck is
useful for this index, then it should be used despite to previsions
guess based on index expression cost.
This is why after setting "is_projection" to false because of too large
cost of index expression, we continue work and check index options for
explicitly specified value.
>
> In RelationGetIndexAttrBitmap(), indexattrs no longer gets the columns
> corresponding to projection indexes. Isn't that weird/error
> prone/confusing? I think it'd be saner to add these bits to both
> bitmaps.
This bitmap is needed only to mark attributes, which modification
prevents hot update.
Should I rename rd_indexattr to rd_hotindexattr or whatever else to
avoid confusion?
Please notice, that rd_indexattr is used only inside
RelationGetIndexAttrBitmap and is not visible from outside.
It is returned for INDEX_ATTR_BITMAP_HOT IndexAttrBitmapKind.
> Please update the comments ending in heapam.c:4188, and generally all
> comments that you should update.
Sorry, did you apply projection-7.patch?
I have checked all trailing spaces and other indentation issues.
At least there is no trailing space at heapam.c:4188...
>
> Please keep serial_schedule in sync with parallel_schedule.
Sorry, once again do not understand your complaint: I have added
func_index both to serial and parallel schedule.
> Also, pgindent.
>
--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company