| From: | Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com> |
|---|---|
| To: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com> |
| Subject: | Re: Proposal: INSERT ... BY NAME |
| Date: | 2026-09-10 08:20:12 |
| Message-ID: | CA+vB=AFBFtHm9rr2kioEC8PXgh9NAQWrg+Cf+EEsr3xvSQpPuw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Ayush,
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().
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.
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.
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.
Regards,
Vaibhav
On Wed, Jul 22, 2026 at 5:42 PM Marcos Pegoraro <marcos(at)f10(dot)com(dot)br> wrote:
> Em qua., 22 de jul. de 2026 às 08:39, Ayush Tiwari <
> ayushtiwari(dot)slg01(at)gmail(dot)com> escreveu:
>
>> Only the *bare* VALUES form is rejected, since a VALUES row has no column
>> names of its own. Maybe I'll reword the docs to make that distinction
>> clear.
>>
>
> yeap, I tested and it works, but it would be good to mention that only
> unnamed values don't work.
>
> regards
> Marcos
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Etsuro Fujita | 2026-09-10 08:39:03 | Re: Further cleanup related to statistics import support in postgres_fdw |
| Previous Message | Andrey Borodin | 2026-09-10 08:12:08 | Re: [PATCH] btree_gist: add cross-type integer operator support for GiST |