introduce CopyFormat, refactor CopyFormatOptions

Currently, the COPY command format is determined by two boolean fields
(binary, csv_mode) in CopyFormatOptions.  This approach, while
functional, isn't ideal for implementing other formats in the future.

To simplify adding new formats, introduce a CopyFormat enum.  This makes
the code cleaner and more maintainable, allowing for easier integration
of additional formats down the line.

Author: Joel Jacobson <joel@compiler.org>
Author: jian he <jian.universality@gmail.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/CALvfUkBxTYy5uWPFVwpk_7ii2zgT07t3d-yR_cy4sfrrLU%3Dkcg%40mail.gmail.com
Discussion: https://postgr.es/m/6a04628d-0d53-41d9-9e35-5a8dc302c34c@joeconway.com

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 63b86802ba25f8624bb8ca9c49217b2dc65cce1e..2f46be516f25a1f3051b4a8599cbf347508b34f6 100644 (file)
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -576,6 +576,8 @@ ProcessCopyOptions(ParseState *pstate,
                opts_out = palloc0_object(CopyFormatOptions);
 
        opts_out->file_encoding = -1;
+       /* default format */
+       opts_out->format = COPY_FORMAT_TEXT;
 
        /* Extract options from the statement node tree */
        foreach(option, options)
@@ -590,11 +592,11 @@ ProcessCopyOptions(ParseState *pstate,
                                errorConflictingDefElem(defel, pstate);
                        format_specified = true;
                        if (strcmp(fmt, "text") == 0)
-                                /* default format */ ;
+                               opts_out->format = COPY_FORMAT_TEXT;
                        else if (strcmp(fmt, "csv") == 0)
-                               opts_out->csv_mode = true;
+                               opts_out->format = COPY_FORMAT_CSV;
                        else if (strcmp(fmt, "binary") == 0)
-                               opts_out->binary = true;
+                               opts_out->format = COPY_FORMAT_BINARY;
                        else
                                ereport(ERROR,
                                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -754,31 +756,31 @@ ProcessCopyOptions(ParseState *pstate,
         * Check for incompatible options (must do these three before inserting
         * defaults)
         */
-       if (opts_out->binary && opts_out->delim)
+       if (opts_out->format == COPY_FORMAT_BINARY && opts_out->delim)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
                                 errmsg("cannot specify %s in BINARY mode", "DELIMITER")));
 
-       if (opts_out->binary && opts_out->null_print)
+       if (opts_out->format == COPY_FORMAT_BINARY && opts_out->null_print)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                                 errmsg("cannot specify %s in BINARY mode", "NULL")));
 
-       if (opts_out->binary && opts_out->default_print)
+       if (opts_out->format == COPY_FORMAT_BINARY && opts_out->default_print)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                                 errmsg("cannot specify %s in BINARY mode", "DEFAULT")));
 
        /* Set defaults for omitted options */
        if (!opts_out->delim)
-               opts_out->delim = opts_out->csv_mode ? "," : "\t";
+               opts_out->delim = (opts_out->format == COPY_FORMAT_CSV) ? "," : "\t";
 
        if (!opts_out->null_print)
-               opts_out->null_print = opts_out->csv_mode ? "" : "\\N";
+               opts_out->null_print = (opts_out->format == COPY_FORMAT_CSV) ? "" : "\\N";
        opts_out->null_print_len = strlen(opts_out->null_print);
 
-       if (opts_out->csv_mode)
+       if (opts_out->format == COPY_FORMAT_CSV)
        {
                if (!opts_out->quote)
                        opts_out->quote = "\"";
@@ -826,7 +828,7 @@ ProcessCopyOptions(ParseState *pstate,
         * future-proofing.  Likewise we disallow all digits though only octal
         * digits are actually dangerous.
         */
-       if (!opts_out->csv_mode &&
+       if (opts_out->format != COPY_FORMAT_CSV &&
                strchr("\\.abcdefghijklmnopqrstuvwxyz0123456789",
                           opts_out->delim[0]) != NULL)
                ereport(ERROR,
@@ -834,43 +836,43 @@ ProcessCopyOptions(ParseState *pstate,
                                 errmsg("COPY delimiter cannot be \"%s\"", opts_out->delim)));
 
        /* Check header */
-       if (opts_out->binary && opts_out->header_line != COPY_HEADER_FALSE)
+       if (opts_out->format == COPY_FORMAT_BINARY && opts_out->header_line != COPY_HEADER_FALSE)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
                                 errmsg("cannot specify %s in BINARY mode", "HEADER")));
 
        /* Check quote */
-       if (!opts_out->csv_mode && opts_out->quote != NULL)
+       if (opts_out->format != COPY_FORMAT_CSV && opts_out->quote != NULL)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
                                 errmsg("COPY %s requires CSV mode", "QUOTE")));
 
-       if (opts_out->csv_mode && strlen(opts_out->quote) != 1)
+       if (opts_out->format == COPY_FORMAT_CSV && strlen(opts_out->quote) != 1)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("COPY quote must be a single one-byte character")));
 
-       if (opts_out->csv_mode && opts_out->delim[0] == opts_out->quote[0])
+       if (opts_out->format == COPY_FORMAT_CSV && opts_out->delim[0] == opts_out->quote[0])
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                 errmsg("COPY delimiter and quote must be different")));
 
        /* Check escape */
-       if (!opts_out->csv_mode && opts_out->escape != NULL)
+       if (opts_out->format != COPY_FORMAT_CSV && opts_out->escape != NULL)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
                                 errmsg("COPY %s requires CSV mode", "ESCAPE")));
 
-       if (opts_out->csv_mode && strlen(opts_out->escape) != 1)
+       if (opts_out->format == COPY_FORMAT_CSV && strlen(opts_out->escape) != 1)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                 errmsg("COPY escape must be a single one-byte character")));
 
        /* Check force_quote */
-       if (!opts_out->csv_mode && (opts_out->force_quote || opts_out->force_quote_all))
+       if (opts_out->format != COPY_FORMAT_CSV && (opts_out->force_quote || opts_out->force_quote_all))
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
@@ -884,8 +886,8 @@ ProcessCopyOptions(ParseState *pstate,
                                                "COPY FROM")));
 
        /* Check force_notnull */
-       if (!opts_out->csv_mode && (opts_out->force_notnull != NIL ||
-                                                               opts_out->force_notnull_all))
+       if (opts_out->format != COPY_FORMAT_CSV && (opts_out->force_notnull != NIL ||
+                                                                                               opts_out->force_notnull_all))
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
@@ -900,8 +902,8 @@ ProcessCopyOptions(ParseState *pstate,
                                                "COPY TO")));
 
        /* Check force_null */
-       if (!opts_out->csv_mode && (opts_out->force_null != NIL ||
-                                                               opts_out->force_null_all))
+       if (opts_out->format != COPY_FORMAT_CSV && (opts_out->force_null != NIL ||
+                                                                                               opts_out->force_null_all))
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                /*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
@@ -925,7 +927,7 @@ ProcessCopyOptions(ParseState *pstate,
                                                "NULL")));
 
        /* Don't allow the CSV quote char to appear in the null string. */
-       if (opts_out->csv_mode &&
+       if (opts_out->format == COPY_FORMAT_CSV &&
                strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -961,7 +963,7 @@ ProcessCopyOptions(ParseState *pstate,
                                                        "DEFAULT")));
 
                /* Don't allow the CSV quote char to appear in the default string. */
-               if (opts_out->csv_mode &&
+               if (opts_out->format == COPY_FORMAT_CSV &&
                        strchr(opts_out->default_print, opts_out->quote[0]) != NULL)
                        ereport(ERROR,
                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -978,7 +980,7 @@ ProcessCopyOptions(ParseState *pstate,
                                         errmsg("NULL specification and DEFAULT specification cannot be the same")));
        }
        /* Check on_error */
-       if (opts_out->binary && opts_out->on_error != COPY_ON_ERROR_STOP)
+       if (opts_out->format == COPY_FORMAT_BINARY && opts_out->on_error != COPY_ON_ERROR_STOP)
                ereport(ERROR,
                                (errcode(ERRCODE_SYNTAX_ERROR),
                                 errmsg("only ON_ERROR STOP is allowed in BINARY mode")));

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 7516b69d01602277e299ac496d1bee0e5d98e943..aa253b587aabc93bddfca107d10d1a83805cc502 100644 (file)
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -157,9 +157,9 @@ static const CopyFromRoutine CopyFromRoutineBinary = {
 static const CopyFromRoutine *
 CopyFromGetRoutine(const CopyFormatOptions *opts)
 {
-       if (opts->csv_mode)
+       if (opts->format == COPY_FORMAT_CSV)
                return &CopyFromRoutineCSV;
-       else if (opts->binary)
+       else if (opts->format == COPY_FORMAT_BINARY)
                return &CopyFromRoutineBinary;
 
        /* default is text */
@@ -263,7 +263,7 @@ CopyFromErrorCallback(void *arg)
                                   cstate->cur_relname);
                return;
        }
-       if (cstate->opts.binary)
+       if (cstate->opts.format == COPY_FORMAT_BINARY)
        {
                /* can't usefully display the data */
                if (cstate->cur_attname)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index ed184ed8e9630e31ee074d62996ea9427c80b2cd..65fd5a0ab4f9dd7ae54423b7e499d278bdacc003 100644 (file)
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -175,7 +175,7 @@ ReceiveCopyBegin(CopyFromState cstate)
 {
        StringInfoData buf;
        int                     natts = list_length(cstate->attnumlist);
-       int16           format = (cstate->opts.binary ? 1 : 0);
+       int16           format = (cstate->opts.format == COPY_FORMAT_BINARY ? 1 : 0);
        int                     i;
 
        pq_beginmessage(&buf, PqMsg_CopyInResponse);
@@ -753,7 +753,7 @@ bool
 NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
 {
        return NextCopyFromRawFieldsInternal(cstate, fields, nfields,
-                                                                                cstate->opts.csv_mode);
+                                                                                cstate->opts.format == COPY_FORMAT_CSV);
 }
 
 /*
@@ -780,7 +780,8 @@ NextCopyFromRawFieldsInternal(CopyFromState cstate, char ***fields, int *nfields
        bool            done = false;
 
        /* only available for text or csv input */
-       Assert(!cstate->opts.binary);
+       Assert(cstate->opts.format == COPY_FORMAT_TEXT ||
+                  cstate->opts.format == COPY_FORMAT_CSV);
 
        /* on input check that the header line is correct if needed */
        if (cstate->cur_lineno == 0 && cstate->opts.header_line != COPY_HEADER_FALSE)

diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 499ce9ad3db8d687bcb8afde2d3b528b50694fd7..3593cb49bf0e88c5a9796eb8bdf2c3bfb5131ae5 100644 (file)
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -183,9 +183,9 @@ static const CopyToRoutine CopyToRoutineBinary = {
 static const CopyToRoutine *
 CopyToGetRoutine(const CopyFormatOptions *opts)
 {
-       if (opts->csv_mode)
+       if (opts->format == COPY_FORMAT_CSV)
                return &CopyToRoutineCSV;
-       else if (opts->binary)
+       else if (opts->format == COPY_FORMAT_BINARY)
                return &CopyToRoutineBinary;
 
        /* default is text */
@@ -222,7 +222,7 @@ CopyToTextLikeStart(CopyToState cstate, TupleDesc tupDesc)
 
                        colname = NameStr(TupleDescAttr(tupDesc, attnum - 1)->attname);
 
-                       if (cstate->opts.csv_mode)
+                       if (cstate->opts.format == COPY_FORMAT_CSV)
                                CopyAttributeOutCSV(cstate, colname, false);
                        else
                                CopyAttributeOutText(cstate, colname);
@@ -399,7 +399,7 @@ SendCopyBegin(CopyToState cstate)
 {
        StringInfoData buf;
        int                     natts = list_length(cstate->attnumlist);
-       int16           format = (cstate->opts.binary ? 1 : 0);
+       int16           format = (cstate->opts.format == COPY_FORMAT_BINARY ? 1 : 0);
        int                     i;
 
        pq_beginmessage(&buf, PqMsg_CopyOutResponse);

diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 877202af67b59748c038100a9a48db2b858c4082..2430fb0b2e594d93438059e659d3d4f807ce9e02 100644 (file)
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -49,6 +49,16 @@ typedef enum CopyLogVerbosityChoice
        COPY_LOG_VERBOSITY_VERBOSE, /* logs additional messages */
 } CopyLogVerbosityChoice;
 
+/*
+ * Represents the format of the COPY operation.
+ */
+typedef enum CopyFormat
+{
+       COPY_FORMAT_TEXT = 0,
+       COPY_FORMAT_BINARY,
+       COPY_FORMAT_CSV,
+} CopyFormat;
+
 /*
  * A struct to hold COPY options, in a parsed form. All of these are related
  * to formatting, except for 'freeze', which doesn't really belong here, but
@@ -59,9 +69,8 @@ typedef struct CopyFormatOptions
        /* parameters from the COPY command */
        int                     file_encoding;  /* file or remote side's character encoding,
                                                                 * -1 if not specified */
-       bool            binary;                 /* binary format? */
+       CopyFormat      format;                 /* format of the COPY operation */
        bool            freeze;                 /* freeze rows on loading? */
-       bool            csv_mode;               /* Comma Separated Value format? */
        int                     header_line;    /* number of lines to skip or COPY_HEADER_XXX
                                                                 * value (see the above) */
        char       *null_print;         /* NULL marker string (server encoding!) */

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a4a2ed0781695d9aefb26e9b2848fab226cfacd3..e1565329f1cc49fb18496bda1ab0d245cd0b4b9a 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -530,6 +530,7 @@ ConversionLocation
 ConvertRowtypeExpr
 CookedConstraint
 CopyDest
+CopyFormat
 CopyFormatOptions
 CopyFromRoutine
 CopyFromState