On Mon, Mar 29, 2021 at 11:41 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Langote <amitlangot...@gmail.com> writes: > > On Sun, Mar 28, 2021 at 1:30 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I wanted to give a data dump of where I am. I've reviewed and > >> nontrivially modified 0001 and the executor parts of 0002, and > >> I'm fairly happy with the state of that much of the code now. > > > Thanks a lot for that work. I have looked at the changes and I agree > > that updateColnosLists + ExecBuildUpdateProjection() looks much better > > than updateTargetLists in the original patch. Looking at > > ExecBuildUpdateProjection(), I take back my comment upthread regarding > > the performance characteristics of your approach, that the prepared > > statements would suffer from having to build the update-new-tuple > > projection(s) from scratch on every execution. > > Yeah, I don't see any reason why the custom projection-build code > would be any slower than the regular path. Related to this, though, > I was wondering whether we could get a useful win by having > nodeModifyTable.c be lazier about doing the per-target-table > initialization steps. > > I think we have to open and lock all the > tables at start for semantic reasons, so maybe that swamps everything > else. But we could avoid purely-internal setup steps, such as > building the slots and projection expressions, until the first time > a particular target is actually updated into. This'd help if we've > failed to prune a lot of partitions that the update/delete won't > actually affect.
Oh, that is exactly what I have proposed in: https://commitfest.postgresql.org/32/2621/ > >> More abstractly, I really dislike the "fake variable" design, primarily > >> the aspect that you made the fake variables look like real columns of > >> the parent table with attnums just beyond the last real one. I think > >> this is just a recipe for obscuring bugs, since it means you have to > >> lobotomize a lot of bad-attnum error checks. The alternative I'm > >> considering is to invent a separate RTE that holds all the junk columns. > >> Haven't tried that yet either. > > > Hmm, I did expect to hear a strong critique of that piece of code. I > > look forward to reviewing your alternative implementation. > > I got one version working over the weekend, but I didn't like the amount > of churn it forced in postgres_fdw (and, presumably, other FDWs). Gimme > a day or so to try something else. Sure, thanks again. -- Amit Langote EDB: http://www.enterprisedb.com
