"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > Please see the version 14 of HOT patch attached.
I expected to find either a large new README, or some pretty substantial additions to existing README files, to document how this all works. The comments included do not represent nearly enough documentation. One thing I was unable to discern from the comments is how CREATE INDEX can work at all. A new index might mean that tuples that could formerly legally be part of the same hot-chain no longer can. I can't find any code here that looks like it's addressing that. I also don't think I believe the reasoning for not indexing DELETE_IN_PROGRESS hot-updated tuples: what if the index completion commits, but the concurrent delete rolls back? Then you've got a valid tuple that's not in the index. > I also took this opportunity to remove the modularity invasion caused > by heap_check_idxupdate() since it was using resultRelInfo. We now > build the list of attributes that must be checked to satisfy HOT update. > This list includes all the index columns, columns in the partial index > predicates and expression index expressions and is built in the > executor. The executor is the wrong place for that: I'm not sure why you think that making heapam depend on the executor is a modularity improvement. Furthermore this approach requires recalculating the list during each query, which is wasteful when it could only change during schema updates. I'd suggest making the relcache responsible for computing and saving this data, along the same lines as RelationGetIndexList(). (That also means that heapam can get the data from the relcache, saving a lot of API-bloating from passing around Attrids explicitly.) Also, rather than inventing Attrids, I'd suggest just using one Bitmapset with the convention that its contents are offset by FirstLowInvalidHeapAttributeNumber. The redefinition of the value of MaxHeapTuplesPerPage seems pretty ugly and unprincipled. I think it'd be better to leave it as-is, and just enforce that we don't allow more than that many line pointers on a heap page (as indeed you have to do anyway, so it's not clear what the change is buying). Is it really necessary to add hot_update to xl_heap_update? Seems the information should be available from the tuple header fields. Have you demonstrated that the "prune_hard" logic is worth its weight? Considering that many of us want to drop VACUUM FULL, adding more complexity to it doesn't seem like a profitable use of time. Is it really safe, or productive, to run heap_page_prune when the buffer is not locked for cleanup (ie, there are other people with pins on it)? Even if it's safe, ISTM what you're mostly accomplishing there is to expend a lot of cycles while holding exclusive lock on the page, when there is good reason to think that you're blocking other people who are interested in using the page. Eliminating the separation between that and cleanup would also allow eliminating the separate "PD_FRAGMENTED" page state. PlanSetValidForThisTransaction is completely bogus --- it's not re-entrant, and it needs to be. I think you need some state in PlannerGlobal instead. rd_avgfsm seems fairly bogus ... when does it get updated? regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match