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


Reply via email to