public inbox for [email protected]
help / color / mirror / Atom feedFrom: Joel Jacobson <[email protected]>
To: [email protected]
Subject: New "raw" COPY format
Date: Fri, 11 Oct 2024 22:29:15 +0200
Message-ID: <[email protected]> (raw)
Hi hackers,
This thread is about implementing a new "raw" COPY format.
This idea came up in a different thread [1], moved here.
[1] https://postgr.es/m/47b5c6a7-5c0e-40aa-8ea2-c7b95ccf296f%40app.fastmail.com
The main use-case for the raw format, is when needing to import arbitrary
unstructured text files, such as log files, into a single text column
of a table.
The name "raw" is just a working title. Andrew had some other good name ideas:
> WFM, so something like FORMAT {SIMPLE, RAW, FAST, SINGLE}?
Below is the draft of its description, sent previously [1],
adjusted thanks to feedback from Daniel Verite, who made me realize the
HEADER option should be made available also for this format.
--- START OF DESCRIPTION ---
Raw Format
The "raw" format is used for importing and exporting files containing
unstructured text, where each line is treated as a single field. This format
is ideal when dealing with data that doesn't conform to a structured,
tabular format and lacks delimiters.
Key Characteristics:
- No Field Delimiters:
Each line is considered a complete value without any field separation.
- Single Column Requirement:
The COPY command must specify exactly one column when using the raw format.
Specifying multiple columns will result in an error.
- Literal Data Interpretation:
All characters are taken literally.
There is no special handling for quotes, backslashes, or escape sequences.
- No NULL Distinction:
Empty lines are imported as empty strings, not as NULL values.
Notes:
- Error Handling:
An error will occur if you use the raw format without specifying exactly
one column or if the table has multiple columns and no column list is
provided.
- Data Preservation:
All characters, including whitespace and special characters, are preserved
exactly as they appear in the file.
--- END OF DESCRIPTION ---
After having studied the code that will be affected,
I feel that before making any changes, I would like to try to improve
ProcessCopyOptions, in terms of readability and maintainability, first.
This seems possible by just reorganize it a bit.
It is actually already organized quite nicely, where the code is mostly
organized per-option, but not always, as it sometimes is spread across
different sections.
It seems possible to organize even more of it per-option,
which would make it easier to reason about each option separately.
This seems possible by organizing the checks per option,
under a single if-branch per option, and moving the setting
of defaults per option (when applicable) to the corresponding
else-branch.
This would also avoid setting defaults for options that are not applicable
for a given format, and instead let their initial NULL value remain untouched,
rather than setting unnecessary defaults.
Some of the checks depend on multiple options in an interdependent way,
not belonging to a specific option more than another. I think such checks
would be nice to place at the end under a separate section.
I also think it would be more readable to use the existing bool variables
named [option]_specified, to determine if an option has been set,
rather than relying on the option's default enum value to evaluate to false.
The attached patch implements the above ideas.
I think with these changes, it would be easier to hack on new and existing
copy options and formats.
/Joel
Attachments:
[application/octet-stream] v1-0001-Replace-binary-flags-binary-and-csv_mode-with-format.patch (18.0K, ../[email protected]/2-v1-0001-Replace-binary-flags-binary-and-csv_mode-with-format.patch)
download | inline diff:
From d621bb2fd0d0d6079ec16a92f5c925fd9fa0baaa Mon Sep 17 00:00:00 2001
From: Joel Jakobsson <[email protected]>
Date: Thu, 10 Oct 2024 08:33:33 +0200
Subject: [PATCH 1/2] Replace binary flags `binary` and `csv_mode` with
`format` enum.
---
src/backend/commands/copy.c | 44 ++++++++++++++--------------
src/backend/commands/copyfrom.c | 10 +++----
src/backend/commands/copyfromparse.c | 34 ++++++++++-----------
src/backend/commands/copyto.c | 20 ++++++-------
src/include/commands/copy.h | 13 ++++++--
5 files changed, 65 insertions(+), 56 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 03eb7a4eba..2021300308 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -493,11 +493,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),
@@ -650,36 +650,36 @@ ProcessCopyOptions(ParseState *pstate,
* Check for incompatible options (must do these two 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")));
- 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")));
/* 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 = "\"";
@@ -727,7 +727,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,
@@ -735,43 +735,43 @@ ProcessCopyOptions(ParseState *pstate,
errmsg("COPY delimiter cannot be \"%s\"", opts_out->delim)));
/* Check header */
- if (opts_out->binary && opts_out->header_line)
+ if (opts_out->format == COPY_FORMAT_BINARY && opts_out->header_line)
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 */
@@ -785,7 +785,7 @@ ProcessCopyOptions(ParseState *pstate,
"COPY FROM")));
/* Check force_notnull */
- if (!opts_out->csv_mode && opts_out->force_notnull != NIL)
+ if (opts_out->format != COPY_FORMAT_CSV && opts_out->force_notnull != NIL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
@@ -799,7 +799,7 @@ ProcessCopyOptions(ParseState *pstate,
"COPY TO")));
/* Check force_null */
- if (!opts_out->csv_mode && opts_out->force_null != NIL)
+ if (opts_out->format != COPY_FORMAT_CSV && opts_out->force_null != NIL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*- translator: %s is the name of a COPY option, e.g. ON_ERROR */
@@ -822,7 +822,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),
@@ -858,7 +858,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),
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 9139a40785..46a662465a 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -122,7 +122,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)
@@ -1576,7 +1576,7 @@ BeginCopyFrom(ParseState *pstate,
cstate->raw_buf_index = cstate->raw_buf_len = 0;
cstate->raw_reached_eof = false;
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
{
/*
* If encoding conversion is needed, we need another buffer to hold
@@ -1627,7 +1627,7 @@ BeginCopyFrom(ParseState *pstate,
continue;
/* Fetch the input function and typioparam info */
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
getTypeBinaryInputInfo(att->atttypid,
&in_func_oid, &typioparams[attnum - 1]);
else
@@ -1768,14 +1768,14 @@ BeginCopyFrom(ParseState *pstate,
pgstat_progress_update_multi_param(3, progress_cols, progress_vals);
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
{
/* Read and verify binary header */
ReceiveCopyBinaryHeader(cstate);
}
/* create workspace for CopyReadAttributes results */
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
{
AttrNumber attr_count = list_length(cstate->attnumlist);
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index 654fecb1b1..50bb4b7750 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -163,7 +163,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);
@@ -749,7 +749,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
bool done;
/* only available for text or csv input */
- Assert(!cstate->opts.binary);
+ Assert(cstate->opts.format != COPY_FORMAT_BINARY);
/* on input check that the header line is correct if needed */
if (cstate->cur_lineno == 0 && cstate->opts.header_line)
@@ -766,7 +766,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
{
int fldnum;
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
fldct = CopyReadAttributesCSV(cstate);
else
fldct = CopyReadAttributesText(cstate);
@@ -821,7 +821,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields)
return false;
/* Parse the line into de-escaped field values */
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
fldct = CopyReadAttributesCSV(cstate);
else
fldct = CopyReadAttributesText(cstate);
@@ -865,7 +865,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
MemSet(nulls, true, num_phys_attrs * sizeof(bool));
MemSet(cstate->defaults, false, num_phys_attrs * sizeof(bool));
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
{
char **field_strings;
ListCell *cur;
@@ -906,7 +906,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext,
continue;
}
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
{
if (string == NULL &&
cstate->opts.force_notnull_flags[m])
@@ -1179,7 +1179,7 @@ CopyReadLineText(CopyFromState cstate)
char quotec = '\0';
char escapec = '\0';
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
{
quotec = cstate->opts.quote[0];
escapec = cstate->opts.escape[0];
@@ -1256,7 +1256,7 @@ CopyReadLineText(CopyFromState cstate)
prev_raw_ptr = input_buf_ptr;
c = copy_input_buf[input_buf_ptr++];
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
{
/*
* If character is '\r', we may need to look ahead below. Force
@@ -1295,7 +1295,7 @@ CopyReadLineText(CopyFromState cstate)
}
/* Process \r */
- if (c == '\r' && (!cstate->opts.csv_mode || !in_quote))
+ if (c == '\r' && (cstate->opts.format != COPY_FORMAT_CSV || !in_quote))
{
/* Check for \r\n on first line, _and_ handle \r\n. */
if (cstate->eol_type == EOL_UNKNOWN ||
@@ -1323,10 +1323,10 @@ CopyReadLineText(CopyFromState cstate)
if (cstate->eol_type == EOL_CRNL)
ereport(ERROR,
(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errmsg("literal carriage return found in data") :
errmsg("unquoted carriage return found in data"),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errhint("Use \"\\r\" to represent carriage return.") :
errhint("Use quoted CSV field to represent carriage return.")));
@@ -1340,10 +1340,10 @@ CopyReadLineText(CopyFromState cstate)
else if (cstate->eol_type == EOL_NL)
ereport(ERROR,
(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errmsg("literal carriage return found in data") :
errmsg("unquoted carriage return found in data"),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errhint("Use \"\\r\" to represent carriage return.") :
errhint("Use quoted CSV field to represent carriage return.")));
/* If reach here, we have found the line terminator */
@@ -1351,15 +1351,15 @@ CopyReadLineText(CopyFromState cstate)
}
/* Process \n */
- if (c == '\n' && (!cstate->opts.csv_mode || !in_quote))
+ if (c == '\n' && (cstate->opts.format != COPY_FORMAT_CSV || !in_quote))
{
if (cstate->eol_type == EOL_CR || cstate->eol_type == EOL_CRNL)
ereport(ERROR,
(errcode(ERRCODE_BAD_COPY_FILE_FORMAT),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errmsg("literal newline found in data") :
errmsg("unquoted newline found in data"),
- !cstate->opts.csv_mode ?
+ cstate->opts.format != COPY_FORMAT_CSV ?
errhint("Use \"\\n\" to represent newline.") :
errhint("Use quoted CSV field to represent newline.")));
cstate->eol_type = EOL_NL; /* in case not set yet */
@@ -1371,7 +1371,7 @@ CopyReadLineText(CopyFromState cstate)
* Process backslash, except in CSV mode where backslash is a normal
* character.
*/
- if (c == '\\' && !cstate->opts.csv_mode)
+ if (c == '\\' && cstate->opts.format != COPY_FORMAT_CSV)
{
char c2;
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 463083e645..78531ae846 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -134,7 +134,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);
@@ -191,7 +191,7 @@ CopySendEndOfRow(CopyToState cstate)
switch (cstate->copy_dest)
{
case COPY_FILE:
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
{
/* Default line termination depends on platform */
#ifndef WIN32
@@ -236,7 +236,7 @@ CopySendEndOfRow(CopyToState cstate)
break;
case COPY_FRONTEND:
/* The FE/BE protocol uses \n as newline for all platforms */
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
CopySendChar(cstate, '\n');
/* Dump the accumulated row as one CopyData message */
@@ -771,7 +771,7 @@ DoCopyTo(CopyToState cstate)
bool isvarlena;
Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1);
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
getTypeBinaryOutputInfo(attr->atttypid,
&out_func_oid,
&isvarlena);
@@ -792,7 +792,7 @@ DoCopyTo(CopyToState cstate)
"COPY TO",
ALLOCSET_DEFAULT_SIZES);
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
{
/* Generate header for a binary copy */
int32 tmp;
@@ -833,7 +833,7 @@ DoCopyTo(CopyToState cstate)
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);
@@ -880,7 +880,7 @@ DoCopyTo(CopyToState cstate)
processed = ((DR_copy *) cstate->queryDesc->dest)->processed;
}
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
{
/* Generate trailer for a binary copy */
CopySendInt16(cstate, -1);
@@ -908,7 +908,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
MemoryContextReset(cstate->rowcontext);
oldcontext = MemoryContextSwitchTo(cstate->rowcontext);
- if (cstate->opts.binary)
+ if (cstate->opts.format == COPY_FORMAT_BINARY)
{
/* Binary per-tuple header */
CopySendInt16(cstate, list_length(cstate->attnumlist));
@@ -917,7 +917,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
/* Make sure the tuple is fully deconstructed */
slot_getallattrs(slot);
- if (!cstate->opts.binary)
+ if (cstate->opts.format != COPY_FORMAT_BINARY)
{
bool need_delim = false;
@@ -937,7 +937,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot)
{
string = OutputFunctionCall(&out_functions[attnum - 1],
value);
- if (cstate->opts.csv_mode)
+ if (cstate->opts.format == COPY_FORMAT_CSV)
CopyAttributeOutCSV(cstate, string,
cstate->opts.force_quote_flags[attnum - 1]);
else
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 6f64d97fdd..4b4079db95 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -51,6 +51,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,
+ 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
@@ -61,9 +71,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? */
CopyHeaderChoice header_line; /* header line? */
char *null_print; /* NULL marker string (server encoding!) */
int null_print_len; /* length of same */
--
2.45.1
[application/octet-stream] v1-0002-Reorganize-ProcessCopyOptions-for-clarity-and-consis.patch (19.0K, ../[email protected]/3-v1-0002-Reorganize-ProcessCopyOptions-for-clarity-and-consis.patch)
download | inline diff:
From d5c1a45ee48bfc0f14ea992589809b0da144d755 Mon Sep 17 00:00:00 2001
From: Joel Jakobsson <[email protected]>
Date: Fri, 11 Oct 2024 21:26:22 +0200
Subject: [PATCH 2/2] Reorganize ProcessCopyOptions for clarity and consistent
option handling.
No changes to the function's signature or behavior; the refactoring solely
improves code structure and readability.
Changes:
* Refactored ProcessCopyOptions to improve readability and maintainability
by grouping per-option checks and default assignments into dedicated sections.
This enhances the logical flow and makes it easier to understand how each COPY
option is processed.
* Explicitly set the default format to COPY_FORMAT_TEXT when the FORMAT option
is not specified. Previously, the default was implied due to
zero-initialization, but making it explicit clarifies the default behavior.
* Consistently use boolean specified-variables to determine if an option has
been provided, rather than relying on default values from zero-initialization.
* Added assertions to ensure necessary options are set before performing
dependent checks, explicitly indicating that they have been assigned either
specified or default values.
* Relocated interdependent option validations to a dedicated section for
additional clarity.
---
src/backend/commands/copy.c | 433 ++++++++++++++++++++++--------------
1 file changed, 271 insertions(+), 162 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 2021300308..b4f6d3ee93 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -647,200 +647,260 @@ ProcessCopyOptions(ParseState *pstate,
}
/*
- * Check for incompatible options (must do these two before inserting
- * defaults)
+ * Set default format if not specified.
+ * This isn't strictly necessary since COPY_FORMAT_TEXT is 0 and
+ * opts_out is palloc0'd, but do it for clarity.
*/
- 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 (!format_specified)
+ opts_out->format = COPY_FORMAT_TEXT;
- 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")));
+ /*
+ * Begin per-option checks and set defaults where necessary
+ */
- 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")));
+ /* --- FORMAT option is always allowed; no additional checks needed --- */
- 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")));
+ /* --- FREEZE option --- */
+ if (freeze_specified)
+ {
+ if (!is_from)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
+ second %s is a COPY with direction, e.g. COPY TO */
+ errmsg("COPY %s cannot be used with %s", "FREEZE",
+ "COPY TO")));
+ }
+ else
+ {
+ /* Default is false; no action needed */
+ }
- /* Set defaults for omitted options */
- if (!opts_out->delim)
- opts_out->delim = opts_out->format == COPY_FORMAT_CSV ? "," : "\t";
+ /* --- DELIMITER option --- */
+ if (opts_out->delim)
+ {
+ if (opts_out->format == COPY_FORMAT_BINARY)
+ 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->null_print)
- opts_out->null_print = opts_out->format == COPY_FORMAT_CSV ? "" : "\\N";
- opts_out->null_print_len = strlen(opts_out->null_print);
+ /* Only single-byte delimiter strings are supported. */
+ if (strlen(opts_out->delim) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("COPY delimiter must be a single one-byte character")));
- if (opts_out->format == COPY_FORMAT_CSV)
+ /* Disallow end-of-line characters */
+ if (strchr(opts_out->delim, '\r') != NULL ||
+ strchr(opts_out->delim, '\n') != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY delimiter cannot be newline or carriage return")));
+
+ /*
+ * Disallow unsafe delimiter characters in non-CSV mode. We can't allow
+ * backslash because it would be ambiguous. We can't allow the other
+ * cases because data characters matching the delimiter must be
+ * backslashed, and certain backslash combinations are interpreted
+ * non-literally by COPY IN. Disallowing all lower case ASCII letters is
+ * more than strictly necessary, but seems best for consistency and
+ * future-proofing. Likewise we disallow all digits though only octal
+ * digits are actually dangerous.
+ */
+ if (opts_out->format != COPY_FORMAT_CSV &&
+ strchr("\\.abcdefghijklmnopqrstuvwxyz0123456789",
+ opts_out->delim[0]) != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY delimiter cannot be \"%s\"", opts_out->delim)));
+ }
+ else if (opts_out->format != COPY_FORMAT_BINARY)
{
- if (!opts_out->quote)
- opts_out->quote = "\"";
- if (!opts_out->escape)
- opts_out->escape = opts_out->quote;
+ /* Set default delimiter */
+ opts_out->delim = opts_out->format == COPY_FORMAT_CSV ? "," : "\t";
}
- /* Only single-byte delimiter strings are supported. */
- if (strlen(opts_out->delim) != 1)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("COPY delimiter must be a single one-byte character")));
+ /* --- NULL option --- */
+ if (opts_out->null_print)
+ {
+ if (opts_out->format == COPY_FORMAT_BINARY)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot specify %s in BINARY mode", "NULL")));
- /* Disallow end-of-line characters */
- if (strchr(opts_out->delim, '\r') != NULL ||
- strchr(opts_out->delim, '\n') != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COPY delimiter cannot be newline or carriage return")));
+ /* Disallow end-of-line characters */
+ if (strchr(opts_out->null_print, '\r') != NULL ||
+ strchr(opts_out->null_print, '\n') != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY null representation cannot use newline or carriage return")));
+ }
+ else if (opts_out->format != COPY_FORMAT_BINARY)
+ {
+ /* Set default null_print */
+ opts_out->null_print = opts_out->format == COPY_FORMAT_CSV ? "" : "\\N";
+ }
+ if (opts_out->null_print)
+ opts_out->null_print_len = strlen(opts_out->null_print);
- if (strchr(opts_out->null_print, '\r') != NULL ||
- strchr(opts_out->null_print, '\n') != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COPY null representation cannot use newline or carriage return")));
+ /* --- HEADER option --- */
+ if (header_specified)
+ {
+ if (opts_out->format == COPY_FORMAT_BINARY)
+ 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")));
+ }
+ else
+ {
+ /* Default is false; no action needed */
+ }
- if (opts_out->default_print)
+ /* --- QUOTE option --- */
+ if (opts_out->quote)
{
- opts_out->default_print_len = strlen(opts_out->default_print);
+ if (opts_out->format != COPY_FORMAT_CSV)
+ 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 (strchr(opts_out->default_print, '\r') != NULL ||
- strchr(opts_out->default_print, '\n') != NULL)
+ if (strlen(opts_out->quote) != 1)
ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COPY default representation cannot use newline or carriage return")));
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("COPY quote must be a single one-byte character")));
+ }
+ else if (opts_out->format == COPY_FORMAT_CSV)
+ {
+ /* Set default quote */
+ opts_out->quote = "\"";
}
- /*
- * Disallow unsafe delimiter characters in non-CSV mode. We can't allow
- * backslash because it would be ambiguous. We can't allow the other
- * cases because data characters matching the delimiter must be
- * backslashed, and certain backslash combinations are interpreted
- * non-literally by COPY IN. Disallowing all lower case ASCII letters is
- * more than strictly necessary, but seems best for consistency and
- * future-proofing. Likewise we disallow all digits though only octal
- * digits are actually dangerous.
- */
- if (opts_out->format != COPY_FORMAT_CSV &&
- strchr("\\.abcdefghijklmnopqrstuvwxyz0123456789",
- opts_out->delim[0]) != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COPY delimiter cannot be \"%s\"", opts_out->delim)));
+ /* --- ESCAPE option --- */
+ if (opts_out->escape)
+ {
+ if (opts_out->format != COPY_FORMAT_CSV)
+ 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")));
- /* Check header */
- if (opts_out->format == COPY_FORMAT_BINARY && opts_out->header_line)
- 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")));
+ if (strlen(opts_out->escape) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("COPY escape must be a single one-byte character")));
+ }
+ else if (opts_out->format == COPY_FORMAT_CSV)
+ {
+ /* Set default escape to quote character */
+ opts_out->escape = opts_out->quote;
+ }
- /* Check quote */
- 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")));
+ /* --- FORCE_QUOTE option --- */
+ if (opts_out->force_quote || opts_out->force_quote_all)
+ {
+ if (opts_out->format != COPY_FORMAT_CSV)
+ 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", "FORCE_QUOTE")));
- 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 (is_from)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
+ second %s is a COPY with direction, e.g. COPY TO */
+ errmsg("COPY %s cannot be used with %s", "FORCE_QUOTE",
+ "COPY FROM")));
+ }
+ else
+ {
+ /* No default action needed */
+ }
- 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")));
+ /* --- FORCE_NOT_NULL option --- */
+ if (opts_out->force_notnull || opts_out->force_notnull_all)
+ {
+ if (opts_out->format != COPY_FORMAT_CSV)
+ 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", "FORCE_NOT_NULL")));
- /* Check escape */
- 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 (!is_from)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
+ second %s is a COPY with direction, e.g. COPY TO */
+ errmsg("COPY %s cannot be used with %s", "FORCE_NOT_NULL",
+ "COPY TO")));
+ }
+ else
+ {
+ /* No default action needed */
+ }
- 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")));
+ /* --- FORCE_NULL option --- */
+ if (opts_out->force_null || opts_out->force_null_all)
+ {
+ if (opts_out->format != COPY_FORMAT_CSV)
+ 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", "FORCE_NULL")));
- /* Check force_quote */
- 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 */
- errmsg("COPY %s requires CSV mode", "FORCE_QUOTE")));
- if ((opts_out->force_quote || opts_out->force_quote_all) && is_from)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
- second %s is a COPY with direction, e.g. COPY TO */
- errmsg("COPY %s cannot be used with %s", "FORCE_QUOTE",
- "COPY FROM")));
+ if (!is_from)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
+ second %s is a COPY with direction, e.g. COPY TO */
+ errmsg("COPY %s cannot be used with %s", "FORCE_NULL",
+ "COPY TO")));
+ }
+ else
+ {
+ /* No default action needed */
+ }
- /* Check force_notnull */
- if (opts_out->format != COPY_FORMAT_CSV && opts_out->force_notnull != NIL)
- 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", "FORCE_NOT_NULL")));
- if (opts_out->force_notnull != NIL && !is_from)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
- second %s is a COPY with direction, e.g. COPY TO */
- errmsg("COPY %s cannot be used with %s", "FORCE_NOT_NULL",
- "COPY TO")));
+ /* --- ON_ERROR option --- */
+ if (on_error_specified)
+ {
+ 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")));
- /* Check force_null */
- if (opts_out->format != COPY_FORMAT_CSV && opts_out->force_null != NIL)
- 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", "FORCE_NULL")));
+ }
+ else
+ {
+ /* Default is COPY_ON_ERROR_STOP */
+ opts_out->on_error = COPY_ON_ERROR_STOP;
+ }
- if (opts_out->force_null != NIL && !is_from)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
- second %s is a COPY with direction, e.g. COPY TO */
- errmsg("COPY %s cannot be used with %s", "FORCE_NULL",
- "COPY TO")));
+ /* --- DEFAULT option --- */
+ if (opts_out->default_print)
+ {
+ if (opts_out->format == COPY_FORMAT_BINARY)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("cannot specify %s in BINARY mode", "DEFAULT")));
- /* Don't allow the delimiter to appear in the null string. */
- if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- /*- translator: %s is the name of a COPY option, e.g. NULL */
- errmsg("COPY delimiter character must not appear in the %s specification",
- "NULL")));
+ /* Assert options have been set (defaults applied if not specified) */
+ Assert(opts_out->delim);
+ Assert(opts_out->quote);
+ Assert(opts_out->null_print);
- /* Don't allow the CSV quote char to appear in the null string. */
- if (opts_out->format == COPY_FORMAT_CSV &&
- strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- /*- translator: %s is the name of a COPY option, e.g. NULL */
- errmsg("CSV quote character must not appear in the %s specification",
- "NULL")));
+ opts_out->default_print_len = strlen(opts_out->default_print);
- /* Check freeze */
- if (opts_out->freeze && !is_from)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- /*- translator: first %s is the name of a COPY option, e.g. ON_ERROR,
- second %s is a COPY with direction, e.g. COPY TO */
- errmsg("COPY %s cannot be used with %s", "FREEZE",
- "COPY TO")));
+ if (strchr(opts_out->default_print, '\r') != NULL ||
+ strchr(opts_out->default_print, '\n') != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY default representation cannot use newline or carriage return")));
- if (opts_out->default_print)
- {
if (!is_from)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -874,6 +934,55 @@ ProcessCopyOptions(ParseState *pstate,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("NULL specification and DEFAULT specification cannot be the same")));
}
+ else
+ {
+ /* No default for default_print; remains NULL */
+ }
+
+ /*
+ * Additional checks for interdependent options
+ */
+
+ /* Checks specific to the CSV and TEXT formats */
+ if (opts_out->format == COPY_FORMAT_TEXT ||
+ opts_out->format == COPY_FORMAT_CSV)
+ {
+ /* Assert options have been set (defaults applied if not specified) */
+ Assert(opts_out->delim);
+ Assert(opts_out->null_print);
+
+ /* Don't allow the delimiter to appear in the NULL or DEFAULT strings */
+
+ if (strchr(opts_out->null_print, opts_out->delim[0]) != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ /*- translator: %s is the name of a COPY option, e.g. NULL */
+ errmsg("COPY delimiter character must not appear in the %s specification",
+ "NULL")));
+ }
+
+ /* Checks specific to the CSV format */
+ if (opts_out->format == COPY_FORMAT_CSV)
+ {
+ /* Assert options have been set (defaults applied if not specified) */
+ Assert(opts_out->delim);
+ Assert(opts_out->quote);
+ Assert(opts_out->null_print);
+
+ if (opts_out->delim[0] == opts_out->quote[0])
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("COPY delimiter and quote must be different")));
+
+ /* Don't allow the CSV quote character in the NULL or DEFAULT strings */
+
+ if (strchr(opts_out->null_print, opts_out->quote[0]) != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ /*- translator: %s is the name of a COPY option, e.g. NULL */
+ errmsg("CSV quote character must not appear in the %s specification",
+ "NULL")));
+ }
}
/*
--
2.45.1
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: New "raw" COPY format
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox