From 5e9f8d838c157f8abb2be059cbdf8f0076be4ed6 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Thu, 26 Dec 2024 15:29:44 +0300 Subject: [PATCH] Error for COPY FROM ... on_error, log_verbosity with Citus tables (#7811) PG17 added the new ON_ERROR option for COPY FROM. When this option is specified, COPY skips soft errors and continues copying. Relevant PG commits: -- https://github.com/postgres/postgres/commit/9e2d87011 -- https://github.com/postgres/postgres/commit/b725b7eec I tried it locally with Citus tables. Without further implementation, it doesn't work correctly. Therefore, we error out for now, and add it to future work. PG17 also added log_verbosity option, which controls the amount of messages emitted during processing. This is currently used in COPY FROM when ON_ERROR option is set to ignore. Therefore, we error out for this option as well. Relevant PG17 commit: https://github.com/postgres/postgres/commit/f5a227895 --- src/backend/distributed/commands/multi_copy.c | 41 +++++++++++++++++++ src/test/regress/expected/pg17.out | 23 +++++++++++ src/test/regress/sql/pg17.sql | 17 ++++++++ 3 files changed, 81 insertions(+) diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index bc632e8b7..758e8694f 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -301,6 +301,7 @@ static SelectStmt * CitusCopySelect(CopyStmt *copyStatement); static void CitusCopyTo(CopyStmt *copyStatement, QueryCompletion *completionTag); static int64 ForwardCopyDataFromConnection(CopyOutState copyOutState, MultiConnection *connection); +static void ErrorIfCopyHasOnErrorLogVerbosity(CopyStmt *copyStatement); /* Private functions copied and adapted from copy.c in PostgreSQL */ static void SendCopyBegin(CopyOutState cstate); @@ -2824,6 +2825,44 @@ CopyStatementHasFormat(CopyStmt *copyStatement, char *formatName) } +/* + * ErrorIfCopyHasOnErrorLogVerbosity errors out if the COPY statement + * has on_error option or log_verbosity option specified + */ +static void +ErrorIfCopyHasOnErrorLogVerbosity(CopyStmt *copyStatement) +{ +#if PG_VERSION_NUM >= PG_VERSION_17 + bool log_verbosity = false; + foreach_ptr(DefElem, option, copyStatement->options) + { + if (strcmp(option->defname, "on_error") == 0) + { + ereport(ERROR, (errmsg( + "Citus does not support COPY FROM with ON_ERROR option."))); + } + else if (strcmp(option->defname, "log_verbosity") == 0) + { + log_verbosity = true; + } + } + + /* + * Given that log_verbosity is currently used in COPY FROM + * when ON_ERROR option is set to ignore, it makes more + * sense to error out for ON_ERROR option first. For this reason, + * we don't error out in the previous loop directly. + * Relevant PG17 commit: https://github.com/postgres/postgres/commit/f5a227895 + */ + if (log_verbosity) + { + ereport(ERROR, (errmsg( + "Citus does not support COPY FROM with LOG_VERBOSITY option."))); + } +#endif +} + + /* * ErrorIfMergeInCopy Raises an exception if the MERGE is called in the COPY * where Citus tables are involved, as we don't support this yet @@ -2926,6 +2965,8 @@ ProcessCopyStmt(CopyStmt *copyStatement, QueryCompletion *completionTag, const "Citus does not support COPY FROM with WHERE"))); } + ErrorIfCopyHasOnErrorLogVerbosity(copyStatement); + /* check permissions, we're bypassing postgres' normal checks */ CheckCopyPermissions(copyStatement); CitusCopyFrom(copyStatement, completionTag); diff --git a/src/test/regress/expected/pg17.out b/src/test/regress/expected/pg17.out index ca2f7643f..99ed0c3e2 100644 --- a/src/test/regress/expected/pg17.out +++ b/src/test/regress/expected/pg17.out @@ -1215,6 +1215,29 @@ btree, for table "pg17.tbl" btree, for table "pg17.tbl_25122024" -- End of testing SET STATISTICS DEFAULT +-- COPY ON_ERROR option +-- Error out for Citus tables because we don't support it yet +-- Relevant PG17 commits: +-- https://github.com/postgres/postgres/commit/9e2d87011 +-- https://github.com/postgres/postgres/commit/b725b7eec +CREATE TABLE check_ign_err (n int, m int[], k int); +SELECT create_distributed_table('check_ign_err', 'n'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +COPY check_ign_err FROM STDIN WITH (on_error stop); +ERROR: Citus does not support COPY FROM with ON_ERROR option. +COPY check_ign_err FROM STDIN WITH (ON_ERROR ignore); +ERROR: Citus does not support COPY FROM with ON_ERROR option. +COPY check_ign_err FROM STDIN WITH (on_error ignore, log_verbosity verbose); +ERROR: Citus does not support COPY FROM with ON_ERROR option. +COPY check_ign_err FROM STDIN WITH (log_verbosity verbose, on_error ignore); +ERROR: Citus does not support COPY FROM with ON_ERROR option. +COPY check_ign_err FROM STDIN WITH (log_verbosity verbose); +ERROR: Citus does not support COPY FROM with LOG_VERBOSITY option. +-- End of Test for COPY ON_ERROR option \set VERBOSITY terse SET client_min_messages TO WARNING; DROP SCHEMA pg17 CASCADE; diff --git a/src/test/regress/sql/pg17.sql b/src/test/regress/sql/pg17.sql index aad8445ea..2018d8b3b 100644 --- a/src/test/regress/sql/pg17.sql +++ b/src/test/regress/sql/pg17.sql @@ -604,6 +604,23 @@ ALTER INDEX tbl_idx ALTER COLUMN 2 SET STATISTICS -1; -- End of testing SET STATISTICS DEFAULT +-- COPY ON_ERROR option +-- Error out for Citus tables because we don't support it yet +-- Relevant PG17 commits: +-- https://github.com/postgres/postgres/commit/9e2d87011 +-- https://github.com/postgres/postgres/commit/b725b7eec + +CREATE TABLE check_ign_err (n int, m int[], k int); +SELECT create_distributed_table('check_ign_err', 'n'); + +COPY check_ign_err FROM STDIN WITH (on_error stop); +COPY check_ign_err FROM STDIN WITH (ON_ERROR ignore); +COPY check_ign_err FROM STDIN WITH (on_error ignore, log_verbosity verbose); +COPY check_ign_err FROM STDIN WITH (log_verbosity verbose, on_error ignore); +COPY check_ign_err FROM STDIN WITH (log_verbosity verbose); + +-- End of Test for COPY ON_ERROR option + \set VERBOSITY terse SET client_min_messages TO WARNING; DROP SCHEMA pg17 CASCADE;