Re: Pluggable Storage - Andres's take
От | Andres Freund |
---|---|
Тема | Re: Pluggable Storage - Andres's take |
Дата | |
Msg-id | 20190321181557.o7sx6ul47t2ojjcm@alap3.anarazel.de обсуждение исходный текст |
Ответ на | Re: Pluggable Storage - Andres's take (Haribabu Kommi <kommi.haribabu@gmail.com>) |
Ответы |
Re: Pluggable Storage - Andres's take
(Haribabu Kommi <kommi.haribabu@gmail.com>)
Re: Pluggable Storage - Andres's take (Andres Freund <andres@anarazel.de>) Re: Pluggable Storage - Andres's take (Andres Freund <andres@anarazel.de>) |
Список | pgsql-hackers |
Hi, Attached is a version of just the first patch. I'm still updating it, but it's getting closer to commit: - There were no tests testing EPQ interactions with DELETE, and only an accidental test for EPQ in UPDATE with a concurrent DELETE. I've added tests. Plan to commit that ahead of the big change. - I was pretty unhappy about how the EPQ integration looked before, I've changed that now. I still wonder if we should restore EvalPlanQualFetch and move the table_lock_tuple() calls in ExecDelete/Update into it. But it seems like it'd not gain that much, because there's custom surrounding code, and it's not that much code. - I changed heapam_tuple_tuple to return *WouldBlock rather than just the last result. I think that's one of the reason Haribabu had neutered a few asserts. - I moved comments from heapam.h to tableam.h where appropriate - I updated the name of HeapUpdateFailureData to TM_FailureData, HTSU_Result to TM_Result, TM_Results members now properly distinguish between update vs modifications (delete & update). - I separated the HEAP_INSERT_ flags into TABLE_INSERT_* and HEAP_INSERT_ with the latter being a copy of TABLE_INSERT_ with the sole addition of _SPECULATIVE. table_insert_speculative callers now don't specify that anymore. Pending work: - Wondering if table_insert/delete/update should rather be table_tuple_insert etc. Would be a bit more consistent with the callback names, but a bigger departure from existing code. - I'm not yet happy with TableTupleDeleted computation in heapam.c, I want to revise that further - formatting - commit message - a few comments need a bit of polishing (ExecCheckTIDVisible, heapam_tuple_lock) - Rename TableTupleMayBeModified to TableTupleOk, but also probably a s/TableTuple/TableMod/ - I'll probably move TUPLE_LOCK_FLAG_LOCK_* into tableam.h - two more passes through the patch On 2019-03-21 15:07:04 +1100, Haribabu Kommi wrote: > As you are modifying the 0003 patch for modify API's, I went and reviewed > the > existing patch and found couple corrections that are needed, in case if you > are not > taken care of them already. Some I have... > + /* Update the tuple with table oid */ > + slot->tts_tableOid = RelationGetRelid(relation); > + if (slot->tts_tableOid != InvalidOid) > + tuple->t_tableOid = slot->tts_tableOid; > > The setting of slot->tts_tableOid is not required in this function, > After set the check is happening, the above code is present in both > heapam_heap_insert and heapam_heap_insert_speculative. I'm not following? Those functions are independent? > + slot->tts_tableOid = RelationGetRelid(relation); > > In heapam_heap_update, i don't think there is a need to update > slot->tts_tableOid. Why? > + default: > + elog(ERROR, "unrecognized heap_update status: %u", result); > > heap_update --> table_update? > > > + default: > + elog(ERROR, "unrecognized heap_delete status: %u", result); > > same as above? I've fixed that in a number of places. > + /*hari FIXME*/ > + /*Assert(result != HeapTupleUpdated && hufd.traversed);*/ > > Removing the commented codes in both ExecDelete and ExecUpdate functions. I don't think that's the right fix. I've refactored that code significantly now, and restored the assert in a, imo, correct version. > + /**/ > + if (epqreturnslot) > + { > + *epqreturnslot = epqslot; > + return NULL; > + } > > comment update missed? Well, you'd deleted a comment around there ;). I've added something back now... Greetings, Andres Freund
Вложения
В списке pgsql-hackers по дате отправления:
Следующее
От: Shawn DebnathДата:
Сообщение: Re: Introduce timeout capability for ConditionVariableSleep