Hello, On Thu, Jul 20, 2023 at 4:06 PM Zhang Mingli <zmlpostg...@gmail.com> wrote: > > Hi, > > On Jul 7, 2023 at 18:00 +0800, Damir Belyalov <dam.be...@gmail.com>, wrote: > > > V2 patch still have some errors when apply file doc/src/sgml/ref/copy.sgml, > rebased and fixed it in V3 path. > Thanks a lot for review. > > I have updated https://commitfest.postgresql.org/43/3896/ to staus Ready for > Committer, thanks again.
I've looked at this patch and it looks mostly fine, though I do not intend to commit it myself; perhaps Andrew will. A few minor things to improve: + If <literal>*</literal> is specified, it will be applied in all columns. ... + If <literal>*</literal> is specified, it will be applied in all columns. Please write "it will be applied in" as "the option will be applied to". + bool force_notnull_all; /* FORCE_NOT_NULL * */ ... + bool force_null_all; /* FORCE_NULL * */ Like in the comment for force_quote, please add a "?" after * in the above comments. + if (cstate->opts.force_notnull_all) + MemSet(cstate->opts.force_notnull_flags, true, num_phys_attrs * sizeof(bool)); ... + if (cstate->opts.force_null_all) + MemSet(cstate->opts.force_null_flags, true, num_phys_attrs * sizeof(bool)); While I am not especially opposed to using this 1-line variant to set the flags array, it does mean that there are now different styles being used for similar code, because force_quote_flags uses a for loop: if (cstate->opts.force_quote_all) { int i; for (i = 0; i < num_phys_attrs; i++) cstate->opts.force_quote_flags[i] = true; } Perhaps we could fix the inconsistency by changing the force_quote_all code to use MemSet() too. I'll defer whether to do that to Andrew's judgement. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com