[feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
Started by Zhang Mingliover 3 years ago14 messages
Hi,
The previous discussion is:
/messages/by-id/CACJufxEnVqzOFtqhexF2+AwOKFrV8zHOY3y=p+gPK6eB14pn_w@mail.gmail.com
We have FORCE_NULL/FORCE_NOT_NULL options when COPY FROM, but users must set the columns one by one.
CREATE TABLE forcetest (
a INT NOT NULL,
b TEXT NOT NULL,
c TEXT,
d TEXT,
e TEXT
);
\pset null NULL
BEGIN;
COPY forcetest (a, b, c, d) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(c,d), FORCE_NULL(c,d));
1,'a',,""
\.
COMMIT;
SELECT c, d FROM forcetest WHERE a = 1;
c | d
---+------
| NULL
(1 row)
We don’t have FORCE_NULL * or FORCE_NOT_NULL * for all columns of a table like FORCE_QUOTE *.
They should be helpful if a table have many columns.
This patch enables FORCE_NULL/FORCE_NOT_NULL options to select all columns of a table just like FORCE_QUOTE * (quote all columns).
BEGIN
COPY forcetest (a, b, c, d) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
2,'b',,""
\.
COMMIT;
SELECT c, d FROM forcetest WHERE a = 2;
c | d
---+------
| NULL
(1 row)
Any thoughts?
Regards,
Zhang Mingli
Attachments:
v0001-COPY-FROM-enable-FORCE_NULL-FORCE_NOT_NULL-on-all-columns.patchapplication/octet-streamDownload
From 19180b6a7c2e17ec8d7ad44385f2cab28cd62c5f Mon Sep 17 00:00:00 2001
From: Mingli Zhang <avamingli@gmail.com>
Date: Mon, 1 Aug 2022 21:13:32 +0800
Subject: [PATCH vn] COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
We already have FORCE_NULL/FORCE_NOT_NULL options to force null string
to be converted to NULL or empty string.
But users must set the columns one by one.
CREATE TABLE forcetest (
a INT NOT NULL,
b TEXT NOT NULL,
c TEXT,
d TEXT,
e TEXT
);
\pset null NULL
BEGIN;
COPY forcetest (a, b, c, d) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(c,d), FORCE_NULL(c,d));
1,'a',,""
\.
COMMIT;
SELECT c, d FROM forcetest WHERE a = 1;
c | d
---+------
| NULL
(1 row)
This commit enables FORCE_NULL/FORCE_NOT_NULL options to select
all columns of a table to be converted.
Like FORCE_QUOTE * , the sql format are FORCE_NULL * and FORCE_NOT_NULL *.
FORCE_NULL * and FORCE_NOT_NULL * can be used simultaneously as before.
BEGIN
COPY forcetest (a, b, c, d) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
2,'b',,""
\.
COMMIT;
SELECT c, d FROM forcetest WHERE a = 2;
c | d
---+------
| NULL
(1 row)
---
doc/src/sgml/ref/copy.sgml | 6 ++--
src/backend/commands/copy.c | 12 +++++---
src/backend/commands/copyfrom.c | 16 +++++++++--
src/backend/parser/gram.y | 8 ++++++
src/include/commands/copy.h | 2 ++
src/test/regress/expected/copy2.out | 44 +++++++++++++++++++++++++++++
src/test/regress/sql/copy2.sql | 30 ++++++++++++++++++++
7 files changed, 110 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 8aae711b3b..d20f4494d6 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -40,8 +40,8 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
QUOTE '<replaceable class="parameter">quote_character</replaceable>'
ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
- FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
- FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
+ FORCE_NOT_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
+ FORCE_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
</synopsis>
</refsynopsisdiv>
@@ -336,6 +336,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
In the default case where the null string is empty, this means that
empty values will be read as zero-length strings rather than nulls,
even when they are not quoted.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
@@ -350,6 +351,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
if it has been quoted, and if a match is found set the value to
<literal>NULL</literal>. In the default case where the null string is empty,
this converts a quoted empty string into NULL.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3ac731803b..30bdcc9702 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -490,9 +490,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
- if (opts_out->force_notnull)
+ if (opts_out->force_notnull || opts_out->force_notnull_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_notnull_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_notnull = castNode(List, defel->arg);
else
ereport(ERROR,
@@ -503,9 +505,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_null") == 0)
{
- if (opts_out->force_null)
+ if (opts_out->force_null || opts_out->force_null_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_null_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_null = castNode(List, defel->arg);
else
ereport(ERROR,
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index a976008b3d..49ba40a8dd 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1249,7 +1249,13 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NOT_NULL name list to per-column flags, check validity */
cstate->opts.force_notnull_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_notnull)
+ if (cstate->opts.force_notnull_all)
+ {
+ int i;
+ for(i = 0; i < num_phys_attrs; i++)
+ cstate->opts.force_notnull_flags[i] = true;
+ }
+ else if (cstate->opts.force_notnull)
{
List *attnums;
ListCell *cur;
@@ -1272,7 +1278,13 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_null)
+ if (cstate->opts.force_null_all)
+ {
+ int i;
+ for(i = 0; i < num_phys_attrs; i++)
+ cstate->opts.force_null_flags[i] = true;
+ }
+ else if (cstate->opts.force_null)
{
List *attnums;
ListCell *cur;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index f9037761f9..8b417b6c4f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3510,10 +3510,18 @@ copy_opt_item:
{
$$ = makeDefElem("force_not_null", (Node *) $4, @1);
}
+ | FORCE NOT NULL_P '*'
+ {
+ $$ = makeDefElem("force_not_null", (Node *) makeNode(A_Star), @1);
+ }
| FORCE NULL_P columnList
{
$$ = makeDefElem("force_null", (Node *) $3, @1);
}
+ | FORCE NULL_P '*'
+ {
+ $$ = makeDefElem("force_null", (Node *) makeNode(A_Star), @1);
+ }
| ENCODING Sconst
{
$$ = makeDefElem("encoding", (Node *) makeString($2), @1);
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index cb0096aeb6..de5edf7584 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -54,8 +54,10 @@ typedef struct CopyFormatOptions
bool force_quote_all; /* FORCE_QUOTE *? */
bool *force_quote_flags; /* per-column CSV FQ flags */
List *force_notnull; /* list of column names */
+ bool force_notnull_all; /* FORCE_NOT_NULL * */
bool *force_notnull_flags; /* per-column CSV FNN flags */
List *force_null; /* list of column names */
+ bool force_null_all; /* FORCE_NULL * */
bool *force_null_flags; /* per-column CSV FN flags */
bool convert_selectively; /* do selective binary conversion? */
List *convert_select; /* list of column names (can be NIL) */
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 5f3685e9ef..b4ea80a268 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -503,6 +503,50 @@ BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ERROR: FORCE_NULL column "b" not referenced by COPY
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+ b | c
+---+------
+ | NULL
+(1 row)
+
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+ b | c
+---+---
+ |
+(1 row)
+
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+ b | c
+---+------
+ b | NULL
+(1 row)
+
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ...c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_...
+ ^
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ... b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL...
+ ^
+ROLLBACK;
\pset null ''
-- test case with whole-row Var in a check constraint
create table check_con_tbl (f1 int);
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index b3c16af48e..528c36f92c 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -334,6 +334,36 @@ ROLLBACK;
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+4,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+5,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+6,"b",""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ROLLBACK;
+
\pset null ''
-- test case with whole-row Var in a check constraint
--
2.34.1
Import Notes
Reply to msg id not found: 49a01964-abea-4bfb-ad97-db49271be559@SparkReference msg id not found: 49a01964-abea-4bfb-ad97-db49271be559@Spark
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
On 2022-08-01 Mo 09:56, Zhang Mingli wrote:
Hi,
The previous discussion is:
/messages/by-id/CACJufxEnVqzOFtqhexF2+AwOKFrV8zHOY3y=p+gPK6eB14pn_w@mail.gmail.com
Starting a new thread is pointless and annoying. As I said in the
previous thread, we would need a patch.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
On 2022-08-01 Mo 15:50, Andrew Dunstan wrote:
On 2022-08-01 Mo 09:56, Zhang Mingli wrote:
Hi,
The previous discussion is:
/messages/by-id/CACJufxEnVqzOFtqhexF2+AwOKFrV8zHOY3y=p+gPK6eB14pn_w@mail.gmail.com
Starting a new thread is pointless and annoying. As I said in the
previous thread, we would need a patch.
Apologies, I se you have sent a patch. I will check it out.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
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);
Thanks,
--
Melih Mutlu
Microsoft
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
HI,
On Dec 27, 2022, 19:02 +0800, Melih Mutlu <m.melihmutlu@gmail.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
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
Hello!
The patch does not work for the current version of postgres, it needs to be
updated.
I tested your patch. Everything looks simple and works well.
There is a suggestion to simplify the code: instead of using
if (cstate->opts.force_notnull_all)
{
int i;
for(i = 0; i < num_phys_attrs; i++)
cstate->opt.force_notnull_flags[i] = true;
}
you can use MemSet():
if (cstate->opts.force_notnull_all)
MemSet(cstate->opt.force_notnull_flags, true, num_phys_attrs *
sizeof(bool));
The same for the force_null case.
Regards,
Damir Belyalov,
Postgres Professional
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
HI,
Regards,
Zhang Mingli
On Jul 7, 2023, 18:00 +0800, Damir Belyalov <dam.bel07@gmail.com>, wrote:
The patch does not work for the current version of postgres, it needs to be updated.
I tested your patch. Everything looks simple and works well.There is a suggestion to simplify the code: instead of using
if (cstate->opts.force_notnull_all)
{
int i;
for(i = 0; i < num_phys_attrs; i++)
cstate->opt.force_notnull_flags[i] = true;
}
Thanks very much for review.
Nice suggestion, patch rebased and updated.
Attachments:
v2-0001-COPY-FROM-enable-FORCE_NULL-FORCE_NOT_NULL-on-all-co.patchapplication/octet-streamDownload
From a416e54e306129d818c9dd4806a7dcc34055d557 Mon Sep 17 00:00:00 2001
From: Zhang Mingli <avamingli@gmail.com>
Date: Sun, 9 Jul 2023 11:19:15 +0800
Subject: [PATCH] COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
We already have FORCE_NULL/FORCE_NOT_NULL options to force null string
to be converted to NULL or empty string.
But users must set the columns one by one.
This commit enables FORCE_NULL/FORCE_NOT_NULL options to select
all columns of a table to be converted.
Like FORCE_QUOTE * , the sql format are FORCE_NULL * and FORCE_NOT_NULL *.
FORCE_NULL * and FORCE_NOT_NULL * can be used simultaneously as before.
BEGIN;
COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
4,,""
\.
COMMIT;
SELECT b, c FROM forcetest WHERE a = 4;
b | c
---+------
| NULL
(1 row)
---
doc/src/sgml/ref/copy.sgml | 6 ++--
src/backend/commands/copy.c | 12 +++++---
src/backend/commands/copyfrom.c | 8 ++++--
src/backend/parser/gram.y | 8 ++++++
src/include/commands/copy.h | 2 ++
src/test/regress/expected/copy2.out | 44 +++++++++++++++++++++++++++++
src/test/regress/sql/copy2.sql | 30 ++++++++++++++++++++
7 files changed, 102 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 5e591ed2e6..4baf62295e 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -40,8 +40,8 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
QUOTE '<replaceable class="parameter">quote_character</replaceable>'
ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
- FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
- FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
+ FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * )
+ FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * )
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
DEFAULT '<replaceable class="parameter">default_string</replaceable>'
</synopsis>
@@ -337,6 +337,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
In the default case where the null string is empty, this means that
empty values will be read as zero-length strings rather than nulls,
even when they are not quoted.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
@@ -351,6 +352,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
if it has been quoted, and if a match is found set the value to
<literal>NULL</literal>. In the default case where the null string is empty,
this converts a quoted empty string into NULL.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f14fae3308..c5d7d78645 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -512,9 +512,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
- if (opts_out->force_notnull)
+ if (opts_out->force_notnull || opts_out->force_notnull_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_notnull_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_notnull = castNode(List, defel->arg);
else
ereport(ERROR,
@@ -525,9 +527,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_null") == 0)
{
- if (opts_out->force_null)
+ if (opts_out->force_null || opts_out->force_null_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_null_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_null = castNode(List, defel->arg);
else
ereport(ERROR,
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 80bca79cd0..23a6e23b4b 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1393,7 +1393,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NOT_NULL name list to per-column flags, check validity */
cstate->opts.force_notnull_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_notnull)
+ if (cstate->opts.force_notnull_all)
+ MemSet(cstate->opts.force_notnull_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_notnull)
{
List *attnums;
ListCell *cur;
@@ -1416,7 +1418,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_null)
+ if (cstate->opts.force_null_all)
+ MemSet(cstate->opts.force_null_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_null)
{
List *attnums;
ListCell *cur;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 39ab7eac0d..7faaa4bbe4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3421,10 +3421,18 @@ copy_opt_item:
{
$$ = makeDefElem("force_not_null", (Node *) $4, @1);
}
+ | FORCE NOT NULL_P '*'
+ {
+ $$ = makeDefElem("force_not_null", (Node *) makeNode(A_Star), @1);
+ }
| FORCE NULL_P columnList
{
$$ = makeDefElem("force_null", (Node *) $3, @1);
}
+ | FORCE NULL_P '*'
+ {
+ $$ = makeDefElem("force_null", (Node *) makeNode(A_Star), @1);
+ }
| ENCODING Sconst
{
$$ = makeDefElem("encoding", (Node *) makeString($2), @1);
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 33175868f6..15bdafd27a 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -56,8 +56,10 @@ typedef struct CopyFormatOptions
bool force_quote_all; /* FORCE_QUOTE *? */
bool *force_quote_flags; /* per-column CSV FQ flags */
List *force_notnull; /* list of column names */
+ bool force_notnull_all; /* FORCE_NOT_NULL * */
bool *force_notnull_flags; /* per-column CSV FNN flags */
List *force_null; /* list of column names */
+ bool force_null_all; /* FORCE_NULL * */
bool *force_null_flags; /* per-column CSV FN flags */
bool convert_selectively; /* do selective binary conversion? */
List *convert_select; /* list of column names (can be NIL) */
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 8e33eee719..dc3929bfee 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -520,6 +520,50 @@ BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ERROR: FORCE_NULL column "b" not referenced by COPY
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+ b | c
+---+------
+ | NULL
+(1 row)
+
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+ b | c
+---+---
+ |
+(1 row)
+
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+ b | c
+---+------
+ b | NULL
+(1 row)
+
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ...c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_...
+ ^
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ... b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL...
+ ^
+ROLLBACK;
\pset null ''
-- test case with whole-row Var in a check constraint
create table check_con_tbl (f1 int);
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index d759635068..a5486f6086 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -344,6 +344,36 @@ ROLLBACK;
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+4,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+5,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+6,"b",""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ROLLBACK;
+
\pset null ''
-- test case with whole-row Var in a check constraint
--
2.34.1
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
Hi,
On Jul 9, 2023 at 11:51 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:
HI,
Regards,
Zhang Mingli
On Jul 7, 2023, 18:00 +0800, Damir Belyalov <dam.bel07@gmail.com>, wrote:
The patch does not work for the current version of postgres, it needs to be updated.
I tested your patch. Everything looks simple and works well.
There is a suggestion to simplify the code: instead of using
if (cstate->opts.force_notnull_all)
{
int i;
for(i = 0; i < num_phys_attrs; i++)
cstate->opt.force_notnull_flags[i] = true;
}
Thanks very much for review.
Nice suggestion, patch rebased and updated.
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.
Zhang Mingli
www.hashdata.xyz
Show quoted text
Attachments:
v3-0001-COPY-FROM-enable-FORCE_NULL-FORCE_NOT_NULL-on-all-co.patchapplication/octet-streamDownload
From 1c42c6c0d1f3af45f1dffa1807b84213eb391f03 Mon Sep 17 00:00:00 2001
From: Zhang Mingli <avamingli@gmail.com>
Date: Wed, 19 Jul 2023 04:57:43 +0800
Subject: [PATCH] COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
We already have FORCE_NULL/FORCE_NOT_NULL options to force null string
to be converted to NULL or empty string.
But users must set the columns one by one.
This commit enables FORCE_NULL/FORCE_NOT_NULL options to select
all columns of a table to be converted.
Like FORCE_QUOTE * , the sql format are FORCE_NULL * and FORCE_NOT_NULL *.
FORCE_NULL * and FORCE_NOT_NULL * can be used simultaneously as before.
BEGIN;
COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
4,,""
\.
COMMIT;
SELECT b, c FROM forcetest WHERE a = 4;
b | c
---+------
| NULL
(1 row)
---
doc/src/sgml/ref/copy.sgml | 6 ++--
src/backend/commands/copy.c | 12 +++++---
src/backend/commands/copyfrom.c | 8 ++++--
src/backend/parser/gram.y | 8 ++++++
src/include/commands/copy.h | 2 ++
src/test/regress/expected/copy2.out | 44 +++++++++++++++++++++++++++++
src/test/regress/sql/copy2.sql | 30 ++++++++++++++++++++
7 files changed, 102 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 4d614a0225..f2005f7d58 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -41,8 +41,8 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
QUOTE '<replaceable class="parameter">quote_character</replaceable>'
ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
- FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
- FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
+ FORCE_NOT_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
+ FORCE_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
</synopsis>
</refsynopsisdiv>
@@ -350,6 +350,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
In the default case where the null string is empty, this means that
empty values will be read as zero-length strings rather than nulls,
even when they are not quoted.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
@@ -364,6 +365,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
if it has been quoted, and if a match is found set the value to
<literal>NULL</literal>. In the default case where the null string is empty,
this converts a quoted empty string into NULL.
+ If <literal>*</literal> is specified, it will be applied in all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f14fae3308..c5d7d78645 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -512,9 +512,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
- if (opts_out->force_notnull)
+ if (opts_out->force_notnull || opts_out->force_notnull_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_notnull_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_notnull = castNode(List, defel->arg);
else
ereport(ERROR,
@@ -525,9 +527,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_null") == 0)
{
- if (opts_out->force_null)
+ if (opts_out->force_null || opts_out->force_null_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_null_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_null = castNode(List, defel->arg);
else
ereport(ERROR,
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 80bca79cd0..23a6e23b4b 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1393,7 +1393,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NOT_NULL name list to per-column flags, check validity */
cstate->opts.force_notnull_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_notnull)
+ if (cstate->opts.force_notnull_all)
+ MemSet(cstate->opts.force_notnull_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_notnull)
{
List *attnums;
ListCell *cur;
@@ -1416,7 +1418,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_null)
+ if (cstate->opts.force_null_all)
+ MemSet(cstate->opts.force_null_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_null)
{
List *attnums;
ListCell *cur;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index edb6c00ece..4ec0173d7f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3418,10 +3418,18 @@ copy_opt_item:
{
$$ = makeDefElem("force_not_null", (Node *) $4, @1);
}
+ | FORCE NOT NULL_P '*'
+ {
+ $$ = makeDefElem("force_not_null", (Node *) makeNode(A_Star), @1);
+ }
| FORCE NULL_P columnList
{
$$ = makeDefElem("force_null", (Node *) $3, @1);
}
+ | FORCE NULL_P '*'
+ {
+ $$ = makeDefElem("force_null", (Node *) makeNode(A_Star), @1);
+ }
| ENCODING Sconst
{
$$ = makeDefElem("encoding", (Node *) makeString($2), @1);
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 33175868f6..15bdafd27a 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -56,8 +56,10 @@ typedef struct CopyFormatOptions
bool force_quote_all; /* FORCE_QUOTE *? */
bool *force_quote_flags; /* per-column CSV FQ flags */
List *force_notnull; /* list of column names */
+ bool force_notnull_all; /* FORCE_NOT_NULL * */
bool *force_notnull_flags; /* per-column CSV FNN flags */
List *force_null; /* list of column names */
+ bool force_null_all; /* FORCE_NULL * */
bool *force_null_flags; /* per-column CSV FN flags */
bool convert_selectively; /* do selective binary conversion? */
List *convert_select; /* list of column names (can be NIL) */
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index faf1a4d1b0..95ec7363af 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -520,6 +520,50 @@ BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ERROR: FORCE_NULL column "b" not referenced by COPY
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+ b | c
+---+------
+ | NULL
+(1 row)
+
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+ b | c
+---+---
+ |
+(1 row)
+
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+ b | c
+---+------
+ b | NULL
+(1 row)
+
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ...c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_...
+ ^
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ... b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL...
+ ^
+ROLLBACK;
\pset null ''
-- test case with whole-row Var in a check constraint
create table check_con_tbl (f1 int);
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index d759635068..a5486f6086 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -344,6 +344,36 @@ ROLLBACK;
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+4,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+5,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+6,"b",""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ROLLBACK;
+
\pset null ''
-- test case with whole-row Var in a check constraint
--
2.36.1
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
Hi,
On Jul 7, 2023 at 18:00 +0800, Damir Belyalov <dam.bel07@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.
Zhang Mingli
www.hashdata.xyz
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
Hello,
On Thu, Jul 20, 2023 at 4:06 PM Zhang Mingli <zmlpostgres@gmail.com> wrote:
Hi,
On Jul 7, 2023 at 18:00 +0800, Damir Belyalov <dam.bel07@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
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
HI,
I've looked at this patch and it looks mostly fine, though I do not
intend to commit it myself; perhaps Andrew will.
HI, Amit, thanks for review.
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".
+1
+ 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.
+1
+ 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.
Sure, let’s wait for Andrew and I will put everything in one pot then.
Zhang Mingli
https://www.hashdata.xyz
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
On 2023-07-26 We 03:03, Zhang Mingli wrote:
HI,
I've looked at this patch and it looks mostly fine, though I do not
intend to commit it myself; perhaps Andrew will.HI, Amit, thanks for review.
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".
+1
+ 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.+1
+ 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.Sure, let’s wait for Andrew and I will put everything in one pot then.
I was hoping it be able to get to it today but that's not happening. If
you want to submit a revised patch as above that will be good. I hope to
get to it later this week.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
On Aug 1, 2023, at 03:35, Andrew Dunstan <andrew@dunslane.net> wrote:
I was hoping it be able to get to it today but that's not happening. If you want to submit a revised patch as above that will be good. I hope to get to it later this week.
HI, Andrew
Patch rebased and updated like above, thanks.

Zhang Mingli
https://www.hashdata.xyz
Attachments:
v4-0001-COPY-FROM-enable-FORCE_NULL-FORCE_NOT_NULL-on-all-co.patchapplication/octet-stream; name=v4-0001-COPY-FROM-enable-FORCE_NULL-FORCE_NOT_NULL-on-all-co.patch; x-unix-mode=0644Download
From 502344e35308dbe9ff22a6b6c181f666dd76b55d Mon Sep 17 00:00:00 2001
From: Zhang Mingli <avamingli@gmail.com>
Date: Tue, 1 Aug 2023 08:34:56 +0800
Subject: [PATCH] COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
We already have FORCE_NULL/FORCE_NOT_NULL options to force null string
to be converted to NULL or empty string.
But users must set the columns one by one.
This commit enables FORCE_NULL/FORCE_NOT_NULL options to select
all columns of a table to be converted.
Like FORCE_QUOTE * , the sql format are FORCE_NULL * and FORCE_NOT_NULL *.
FORCE_NULL * and FORCE_NOT_NULL * can be used simultaneously as before.
BEGIN;
COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
4,,""
\.
COMMIT;
SELECT b, c FROM forcetest WHERE a = 4;
b | c
---+------
| NULL
(1 row)
---
doc/src/sgml/ref/copy.sgml | 6 ++--
src/backend/commands/copy.c | 12 +++++---
src/backend/commands/copyfrom.c | 8 ++++--
src/backend/commands/copyto.c | 5 +---
src/backend/parser/gram.y | 8 ++++++
src/include/commands/copy.h | 2 ++
src/test/regress/expected/copy2.out | 44 +++++++++++++++++++++++++++++
src/test/regress/sql/copy2.sql | 30 ++++++++++++++++++++
8 files changed, 103 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index 4d614a0225..d12ba96497 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -41,8 +41,8 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
QUOTE '<replaceable class="parameter">quote_character</replaceable>'
ESCAPE '<replaceable class="parameter">escape_character</replaceable>'
FORCE_QUOTE { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
- FORCE_NOT_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
- FORCE_NULL ( <replaceable class="parameter">column_name</replaceable> [, ...] )
+ FORCE_NOT_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
+ FORCE_NULL { ( <replaceable class="parameter">column_name</replaceable> [, ...] ) | * }
ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
</synopsis>
</refsynopsisdiv>
@@ -350,6 +350,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
In the default case where the null string is empty, this means that
empty values will be read as zero-length strings rather than nulls,
even when they are not quoted.
+ If <literal>*</literal> is specified, the option will be applied to all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
@@ -364,6 +365,7 @@ COPY { <replaceable class="parameter">table_name</replaceable> [ ( <replaceable
if it has been quoted, and if a match is found set the value to
<literal>NULL</literal>. In the default case where the null string is empty,
this converts a quoted empty string into NULL.
+ If <literal>*</literal> is specified, the option will be applied to all columns.
This option is allowed only in <command>COPY FROM</command>, and only when
using <literal>CSV</literal> format.
</para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f14fae3308..c5d7d78645 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -512,9 +512,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
- if (opts_out->force_notnull)
+ if (opts_out->force_notnull || opts_out->force_notnull_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_notnull_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_notnull = castNode(List, defel->arg);
else
ereport(ERROR,
@@ -525,9 +527,11 @@ ProcessCopyOptions(ParseState *pstate,
}
else if (strcmp(defel->defname, "force_null") == 0)
{
- if (opts_out->force_null)
+ if (opts_out->force_null || opts_out->force_null_all)
errorConflictingDefElem(defel, pstate);
- if (defel->arg && IsA(defel->arg, List))
+ if (defel->arg && IsA(defel->arg, A_Star))
+ opts_out->force_null_all = true;
+ else if (defel->arg && IsA(defel->arg, List))
opts_out->force_null = castNode(List, defel->arg);
else
ereport(ERROR,
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index b47cb5c66d..122d5a766e 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1393,7 +1393,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NOT_NULL name list to per-column flags, check validity */
cstate->opts.force_notnull_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_notnull)
+ if (cstate->opts.force_notnull_all)
+ MemSet(cstate->opts.force_notnull_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_notnull)
{
List *attnums;
ListCell *cur;
@@ -1416,7 +1418,9 @@ BeginCopyFrom(ParseState *pstate,
/* Convert FORCE_NULL name list to per-column flags, check validity */
cstate->opts.force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
- if (cstate->opts.force_null)
+ if (cstate->opts.force_null_all)
+ MemSet(cstate->opts.force_null_flags, true, num_phys_attrs * sizeof(bool));
+ else if (cstate->opts.force_null)
{
List *attnums;
ListCell *cur;
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 9e4b2437a5..7278d6cebc 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -582,10 +582,7 @@ BeginCopyTo(ParseState *pstate,
cstate->opts.force_quote_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
if (cstate->opts.force_quote_all)
{
- int i;
-
- for (i = 0; i < num_phys_attrs; i++)
- cstate->opts.force_quote_flags[i] = true;
+ MemSet(cstate->opts.force_quote_flags, true, num_phys_attrs * sizeof(bool));
}
else if (cstate->opts.force_quote)
{
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 15ece871a0..19badaf962 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -3419,10 +3419,18 @@ copy_opt_item:
{
$$ = makeDefElem("force_not_null", (Node *) $4, @1);
}
+ | FORCE NOT NULL_P '*'
+ {
+ $$ = makeDefElem("force_not_null", (Node *) makeNode(A_Star), @1);
+ }
| FORCE NULL_P columnList
{
$$ = makeDefElem("force_null", (Node *) $3, @1);
}
+ | FORCE NULL_P '*'
+ {
+ $$ = makeDefElem("force_null", (Node *) makeNode(A_Star), @1);
+ }
| ENCODING Sconst
{
$$ = makeDefElem("encoding", (Node *) makeString($2), @1);
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 33175868f6..17ee29dd75 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -56,8 +56,10 @@ typedef struct CopyFormatOptions
bool force_quote_all; /* FORCE_QUOTE *? */
bool *force_quote_flags; /* per-column CSV FQ flags */
List *force_notnull; /* list of column names */
+ bool force_notnull_all; /* FORCE_NOT_NULL *? */
bool *force_notnull_flags; /* per-column CSV FNN flags */
List *force_null; /* list of column names */
+ bool force_null_all; /* FORCE_NULL *? */
bool *force_null_flags; /* per-column CSV FN flags */
bool convert_selectively; /* do selective binary conversion? */
List *convert_select; /* list of column names (can be NIL) */
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index faf1a4d1b0..95ec7363af 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -520,6 +520,50 @@ BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ERROR: FORCE_NULL column "b" not referenced by COPY
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+ b | c
+---+------
+ | NULL
+(1 row)
+
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+ b | c
+---+---
+ |
+(1 row)
+
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+ b | c
+---+------
+ b | NULL
+(1 row)
+
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ...c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_...
+ ^
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ERROR: conflicting or redundant options
+LINE 1: ... b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL...
+ ^
+ROLLBACK;
\pset null ''
-- test case with whole-row Var in a check constraint
create table check_con_tbl (f1 int);
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index d759635068..a5486f6086 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -344,6 +344,36 @@ ROLLBACK;
BEGIN;
COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
ROLLBACK;
+-- should succeed with no effect ("b" remains an empty string, "c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NULL *);
+4,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 4;
+-- should succeed with effect ("b" remains an empty string)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *);
+5,,""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 5;
+-- should succeed with effect ("c" remains NULL)
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *);
+6,"b",""
+\.
+COMMIT;
+SELECT b, c FROM forcetest WHERE a = 6;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL *, FORCE_NOT_NULL(b));
+ROLLBACK;
+-- should fail with "conflicting or redundant options" error
+BEGIN;
+COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL *, FORCE_NULL(b));
+ROLLBACK;
+
\pset null ''
-- test case with whole-row Var in a check constraint
--
2.36.1
Re: [feature]COPY FROM enable FORCE_NULL/FORCE_NOT_NULL on all columns
On 2023-07-31 Mo 20:46, Zhang Mingli wrote:
On Aug 1, 2023, at 03:35, Andrew Dunstan <andrew@dunslane.net> wrote:
I was hoping it be able to get to it today but that's not happening.
If you want to submit a revised patch as above that will be good. I
hope to get to it later this week.HI, Andrew
Patch rebased and updated like above, thanks.
Pushed at last, thanks.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com