Re: Proposal: INSERT ... BY NAME - Mailing list pgsql-hackers

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.dalvi@enterprisedb.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