Separate external table options out of copy option check (#11104)

ProcessCopyOptions checks the option list of COPY command. It's also
called by external table when text/csv format is used. It's better
not to mix external table specific options here and check them
separately.

Checking custom protocol here is not necessary because it's checked
when parsing location urls in GenerateExtTableEntryOptions anyway.

contrib/file_fdw/file_fdw.c

@@ -316,7 +316,7 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 	/*
 	 * Now apply the core COPY code's validation logic for more checks.
 	 */
-	ProcessCopyOptions(NULL, NULL, true, other_options, 0, true);
+	ProcessCopyOptions(NULL, NULL, true, other_options);
 
 	/*
 	 * Either filename or program option is required for file_fdw foreign

gpcontrib/gp_exttable_fdw/extaccess.c

@@ -270,6 +270,12 @@ external_beginscan(Relation relation, uint32 scancounter,
 									NIL,
 									(fmttype_is_custom(fmtType) ? NIL : extOptions));
 
+	if (scan->fs_pstate->header_line && Gp_role == GP_ROLE_DISPATCH)
+	{
+		ereport(NOTICE,
+				(errmsg("HEADER means that each one of the data files has a header row")));
+	}
+
 	/* Initialize all the parsing and state variables */
 	InitParseState(scan->fs_pstate, relation, false, fmtType,
 				   scan->fs_uri, rejLimit, rejLimitInRows, logErrors);

gpcontrib/pxf_fdw/pxf_option.c

@@ -361,7 +361,7 @@ ValidateCopyOptions(List *options_list, Oid catalog)
 	/*
 	 * Apply the core COPY code's validation logic for more checks.
 	 */
-	ProcessCopyOptions(NULL, NULL, true, copy_options, 0, true);
+	ProcessCopyOptions(NULL, NULL, true, copy_options);
 
 	PG_RETURN_VOID();
 }

src/backend/commands/copy.c

@@ -1394,14 +1394,10 @@ void
 ProcessCopyOptions(ParseState *pstate,
 				   CopyState cstate,
 				   bool is_from,
-				   List *options,
-				   int num_columns,
-				   bool is_copy) /* false means external table */
+				   List *options)
 {
 	bool		format_specified = false;
 	ListCell   *option;
-	Oid			extprotocol_oid = InvalidOid;
-	ExtTableEntry *exttbl = NULL;
 
 	/* Support external use for option sanity checking */
 	if (cstate == NULL)
@@ -1415,25 +1411,6 @@ ProcessCopyOptions(ParseState *pstate,
 	cstate->delim_off = false;
 	cstate->file_encoding = -1;
 
-	if (cstate->rel && rel_is_external_table(cstate->rel->rd_id))
-	{
-		is_copy = false;
-		num_columns = cstate->rel->rd_att->natts;
-		exttbl = GetExtTableEntry(cstate->rel->rd_id);
-	}
-
-	if (exttbl && exttbl->urilocations)
-	{
-		char	   *location;
-		char	   *protocol;
-		Size		position;
-
-		location = strVal(linitial(exttbl->urilocations));
-		position = strchr(location, ':') - location;
-		protocol = pnstrdup(location, position);
-		extprotocol_oid = get_extprotocol_oid(protocol, true);
-	}
-
 	/* Extract options from the statement node tree */
 	foreach(option, options)
 	{
@@ -1674,7 +1651,7 @@ ProcessCopyOptions(ParseState *pstate,
 						 errmsg("conflicting or redundant options")));
 			cstate->skip_ext_partition = true;
 		}
-		else if (!extprotocol_oid)
+		else if (!rel_is_external_table(cstate->rel->rd_id))
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
 					 errmsg("option \"%s\" not recognized",
@@ -1877,51 +1854,6 @@ ProcessCopyOptions(ParseState *pstate,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("COPY delimiter cannot be backslash")));
 
-	if (cstate->delim_off)
-	{
-
-		/*
-		 * We don't support delimiter 'off' for COPY because the QD COPY
-		 * sometimes internally adds columns to the data that it sends to
-		 * the QE COPY modules, and it uses the delimiter for it. There
-		 * are ways to work around this but for now it's not important and
-		 * we simply don't support it.
-		 */
-		if (is_copy)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("using no delimiter is only supported for external tables")));
-
-		if (num_columns != 1)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("using no delimiter is only possible for a single column table")));
-
-	}
-
-
-	/* Check header */
-	if (cstate->header_line)
-	{
-		if (!is_copy && Gp_role == GP_ROLE_DISPATCH)
-		{
-			/* (exttab) */
-			if (is_from)
-			{
-				/* RET */
-				ereport(NOTICE,
-						(errmsg("HEADER means that each one of the data files has a header row")));
-			}
-			else
-			{
-				/* WET */
-				ereport(ERROR,
-						(errcode(ERRCODE_GP_FEATURE_NOT_YET),
-						errmsg("HEADER is not yet supported for writable external tables")));
-			}
-		}
-	}
-
 	if (cstate->fill_missing && !is_from)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1989,7 +1921,6 @@ BeginCopy(ParseState *pstate,
 	CopyState	cstate;
 	int			num_phys_attrs;
 	MemoryContext oldcontext;
-	int num_columns = 0;
 
 	/* Allocate workspace and zero all fields */
 	cstate = (CopyStateData *) palloc0(sizeof(CopyStateData));
@@ -2011,9 +1942,21 @@ BeginCopy(ParseState *pstate,
 		cstate->rel = rel;
 
 	/* Extract options from the statement node tree */
-	ProcessCopyOptions(pstate, cstate, is_from, options,
-					   num_columns, /* pass correct value when COPY supports no delim */
-					   true);
+	ProcessCopyOptions(pstate, cstate, is_from, options);
+
+	if (cstate->delim_off && !rel_is_external_table(rel->rd_id))
+	{
+		/*
+		 * We don't support delimiter 'off' for COPY because the QD COPY
+		 * sometimes internally adds columns to the data that it sends to
+		 * the QE COPY modules, and it uses the delimiter for it. There
+		 * are ways to work around this but for now it's not important and
+		 * we simply don't support it.
+		 */
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("using no delimiter is only supported for external tables")));
+	}
 
 	/* Process the source/target relation or query */
 	if (rel)

src/backend/commands/exttablecmds.c

@@ -669,9 +669,35 @@ transformFormatOpts(char formattype, List *formatOpts, int numcols, bool iswrita
 		ProcessCopyOptions(pstate,
 						   cstate,
 						   !iswritable, /* is_from */
-						   formatOpts,
-						   numcols,
-						   false /* is_copy */);
+						   formatOpts);
+
+		if (cstate->delim_off)
+		{
+			if (numcols != 1)
+				ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("using no delimiter is only possible for a single column table")));
+		}
+
+		if (cstate->header_line)
+		{
+			if (Gp_role == GP_ROLE_DISPATCH)
+			{
+				if (!iswritable)
+				{
+					/* RET */
+					ereport(NOTICE,
+							(errmsg("HEADER means that each one of the data files has a header row")));
+				}
+				else
+				{
+					/* WET */
+					ereport(ERROR,
+							(errcode(ERRCODE_GP_FEATURE_NOT_YET),
+							errmsg("HEADER is not yet supported for writable external tables")));
+				}
+			}
+		}
 
 		/* keep the same order with the original pg_exttable catalog's fmtopt field */
 		cslist = lappend(cslist, makeDefElem("delimiter", (Node *) makeString(cstate->delim), -1));

src/include/commands/copy.h

@@ -288,9 +288,8 @@ extern void DoCopy(ParseState *state, const CopyStmt *stmt,
 				   int stmt_location, int stmt_len,
 				   uint64 *processed);
 
-extern void ProcessCopyOptions(ParseState *pstate, CopyState cstate, bool is_from, List *options,
-                               int num_columns,
-                               bool is_copy);
+extern void ProcessCopyOptions(ParseState *pstate, CopyState cstate, bool is_from, List *options);
+
 extern CopyState BeginCopyFrom(ParseState *pstate, Relation rel, const char *filename,
 							   bool is_program, copy_data_source_cb data_source_cb,
 							   void *data_source_cb_extra,