Re: Proposal: INSERT ... BY NAME

Hi Ayush,

All the points from my last review are fixed.

A few small new comments, none are bugs:

1) No tab-completion after "BY NAME"/"BY POSITION" is typed (unlike
OVERRIDING, which does complete SELECT/VALUES/TABLE after it).
2) "INSERT ... BY NAME TABLE other_table" works but has no regression
test.

Regards,
Vaibhav Dalvi
EnterpriseDB

On Sat, Sep 12, 2026 at 9:12 PM Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
wrote:

> Hi,
>
> Thanks for the review.
>
> I've attached v2 that addresses both Marcos and your points.
>
> Marcos wrote:
>
> > it would be good to mention that only unnamed values don't work.
>
> The docs now call the unsupported form a "bare VALUES source", and show
> the named form:
>
> INSERT INTO t1 BY NAME
> SELECT * FROM (VALUES (1, 2)) AS v(c2, c1);
>
> > And is missing a test for select *, right?
>
> Added.
>
> On Thu, 10 Sept 2026 at 13:50, Vaibhav Dalvi
> <vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
>
> > This is a solid patch but here are my observations:
> >
> > 1. The BY NAME + VALUES/DEFAULT VALUES rejection check runs before the
> target
> > table/column are validated, which masks the real error:
> >
> > postgres=# INSERT INTO no_such_table BY NAME VALUES (1,2);
> > ERROR: cannot use BY NAME with VALUES
> > HINT: BY NAME requires a query, such as a SELECT, as the data source.
> >
> > It reports "cannot use BY NAME with VALUES" instead of "relation does
> not exist".
> > This is an ordering bug, easy to fix by moving the check after
> setTargetTable()/checkInsertTargets().
>
> I moved it after setTargetTable() and checkInsertTargets(), so relation and
> attname errors win, but bare VALUES is still rejected before source
> transformation. Thanks for raising this.
>
> > 2. srccolnames list is not needed: It's a separate list built only to
> > carry tle->resname values, but the same names are already available on
> > selectQuery->targetList. transformInsertColsByName could just
> > read them from there directly, one less list to carry around.
>
> Dropped. transformInsertColsByName() now walks selectQuery->targetList,
> skips resjunk TLEs, matches on resname, and uses tle->expr for errposition.
>
> > 3. Grammar has too many hand-written alternatives:
> > insert_rest grows from 5 to 13 almost-identical rules to cover
> > every order of BY NAME/POSITION with OVERRIDING and the column
> > list. Each one repeats the same 3 assignments by hand. The
> > codebase already has a pattern for this kind of thing
> > (opt_unique_null_treatment), worth factoring this the same way so
> > a future change does not need to touch 8 places at once.
>
> I took a narrower route here: the old insert_rest productions stay as-is,
> while insert_by_options covers only the new BY forms. My thought was to
> avoid repeating the semantic actions without routing existing INSERT syntax
> through new grammar. Does that seem like a reasonable boundary?
>
> > 4. matched Bitmapset duplicates new_attrnos:
> > Both matched and new_attrnos are built in the same loop and hold
> > the same information. bms_is_member(x, matched) could just be
> > list_member_int(new_attrnos, x), one less variable to keep in
> > sync.
>
> Dropped. The duplicate check now uses list_member_int(new_attrnos,
> matchattno).
>
> I also added psql completion.
>
> Thoughts on the v2?
>
> Regards,
> Ayush
>