On Sat, Mar 16, 2019 at 5:43 PM Haribabu Kommi <kommi.harib...@gmail.com> wrote:
> > > On Sat, Mar 9, 2019 at 2:13 PM Andres Freund <and...@anarazel.de> wrote: > >> Hi, >> >> While 0001 is pretty bulky, the interesting bits concentrate on a >> comparatively small area. I'd appreciate if somebody could give the >> comments added in tableam.h a read (both on callbacks, and their >> wrappers, as they have different audiences). It'd make sense to first >> read the commit message, to understand the goal (and I'd obviously also >> appreciate suggestions for improvements there as well). >> >> I'm pretty happy with the current state of the scan patch. I plan to do >> two more passes through it (formatting, comment polishing, etc. I don't >> know of any functional changes needed), and then commit it, lest >> somebody objects. >> > > I found couple of typos in the committed patch, attached patch fixes them. > I am not sure about one typo, please check it once. > > And I reviewed the 0002 patch, which is a pretty simple and it can be > committed. > 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. + /* 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. + slot->tts_tableOid = RelationGetRelid(relation); In heapam_heap_update, i don't think there is a need to update slot->tts_tableOid. + 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? + /*hari FIXME*/ + /*Assert(result != HeapTupleUpdated && hufd.traversed);*/ Removing the commented codes in both ExecDelete and ExecUpdate functions. + /**/ + if (epqreturnslot) + { + *epqreturnslot = epqslot; + return NULL; + } comment update missed? Regards, Haribabu Kommi Fujitsu Australia