Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns

HI,

On Dec 27, 2022, 19:02 +0800, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>, wrote:
Hi,

Having  FORCE_NULL(*) and FORCE_NOT_NULL(*) sounds good, since postgres already has FORCE_QUOTE(*).

I just quickly tried out your patch. It worked for me as expected.

 One little suggestion:

+ if (cstate->opts.force_notnull_all)
+ {
+     int i;
+     for(i = 0; i < num_phys_attrs; i++)
+         cstate->opts.force_notnull_flags[i] = true;
+ }

Instead of setting force_null/force_notnull flags for all columns, what about simply setting "attnums" list to cstate->attnumlist?
Something like the following should be enough :
if (cstate->opts.force_null_all)
   attnums = cstate->attnumlist;
else
   attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->opts.force_null);
Tanks very much for review.

I got your point and we have to handle the case that there are no force_* options at all.
So the codes will be like:

```
List *attnums = NIL;

if (cstate->opts.force_notnull_all)
attnums = cstate->attnumlist;
else if (cstate->opts.force_notnull)
attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->opts.force_notnull);

if (attnums != NIL)
{
// process force_notnull columns

attnums = NIL; // to process other options later
}

if (cstate->opts.force_null_all)
attnums = cstate->attnumlist;
else if (cstate->opts.force_null)
attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->opts.force_null);

if (attnums != NIL)
{
// process force_null columns

attnums = NIL; // to process other options later
}
```
That seems a little odd.

Or, we could keep attnums as local variables, then the codes will be like:

```
if (cstate->opts.force_notnull_all || cstate->opts.force_notnull)
{
if (cstate->opts.force_notnull_all)
attnums = cstate->attnumlist;
else
attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->opts.force_notnull);
// process force_notnull columns
}
```

Any other suggestions?

Regards,
Zhang Mingli