Hi Jian
On 25/04/2026 06:12, jian he wrote:
Comments are welcome!
Thanks for the patch!
A few comments:
== double defGet in function name ==
defGetdefGetCopyOnConflictChoice should probably be
defGetCopyOnConflictChoice
== identical error message in ProcessCopyOptions() ==
The same error message is raised for conflict_tbl_specified and
!conflict_tbl_specified
ereport(ERROR,
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("COPY %s requires %s option", "CONFLICT_TABLE", "ON_CONFLICT"));
== unused enum in CopyOnErrorChoice ==
The enum COPY_ON_ERROR_TABLE is introduced, but is never used anywhere.
== unconditional ExecOpenIndices() ==
ExecOpenIndices(resultRelInfo, true);
I'm not very familiar with this part of the code, but it looks like that
this change would affect other COPY FROM operations. If I'm mistaken, a
comment would add some value here. Or perhaps something like:
ExecOpenIndices(resultRelInfo, cstate->opts.on_conflict != ONCONFLICT_NONE);
== ON_CONFLICT TABLE not rejected in COPY TO ==
CONFLICT_TABLE is silently ignored, even if the table does not exist:
postgres=# COPY t TO '/dev/null' (ON_CONFLICT TABLE, CONFLICT_TABLE
table_does_not_exist);
COPY 1
I guess adding a is_from to defGetdefGetCopyOnConflictChoice() is the
way to go.
== redundant condition in CopyFrom() ==
The second condition seems unnecessary, as the previous if already tests
for cstate->opts.on_conflict == ONCONFLICT_NONE:
else if (resultRelInfo->ri_NumIndices > 0 &&
cstate->opts.on_conflict != ONCONFLICT_NONE)
== typos ==
regular realtion > regular relation
vertification > verification
resouces > resources
unqiue > unique
== unnecessary pnstrdup (?) ==
newvalues[i] = CStringGetTextDatum(pnstrdup(cstate->line_buf.data,
cstate->line_buf.len));
Is the duplication really necessary? Wouldn't it suffice to use
cstring_to_text_with_len() instead? Something like:
newvalues[i] =
PointerGetDatum(cstring_to_text_with_len(cstate->line_buf.data,
cstate->line_buf.len));
or even
newvalues[i] = CStringGetTextDatum(cstate->line_buf.data)
I'll check the documentation after we get more feedback on the syntax.
Thanks!
Best, Jim
