пн, 26 окт. 2020 г. в 22:15, Peter Geoghegan <p...@bowt.ie>: > Attached is v5, which has changes that are focused on two important > high level goals: >
And some more comments after another round of reading the patch. 1. Looks like UNIQUE_CHECK_NO_WITH_UNCHANGED is used for HOT updates, should we use UNIQUE_CHECK_NO_HOT here? It is better understood like this. 2. You're modifying the table_tuple_update() function on 1311 line of include/access/tableam.h, adding modified_attrs_hint. There's a large comment right before it describing parameters, I think there should be a note about modified_attrs_hint parameter in that comment, 'cos it is referenced from other places in tableam.h and also from backend/access/heap/heapam.c 3. Can you elaborate on the scoring model you're using? Why do we expect a score of 25, what's the rationale behind this number? And should it be #define-d ? 4. heap_compute_xid_horizon_for_tuples contains duplicate logic. Is it possible to avoid this? 5. In this comment + * heap_index_batch_check() helper function. Sorts deltids array in the + * order needed for useful processing. perhaps it is better to replace "useful" with more details? Or point to the place where "useful processing" is described. 6. In this snippet in _bt_dedup_delete_pass() + else if (_bt_keep_natts_fast(rel, state->base, itup) > nkeyatts && + _bt_dedup_save_htid(state, itup)) + { + + } I would rather add a comment, explaining that the empty body of the clause is actually expected. 7. In the _bt_dedup_delete_finish_pending() you're setting ispromising to false for both posting and non-posting tuples. This contradicts comments before function. -- Victor Yegorov