Re: Proposal: INSERT ... BY NAME

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