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,
