2014-03-02 8:26 GMT+09:00 Andrew Dunstan <and...@dunslane.net>:
>
> On 01/29/2014 10:59 AM, Ian Lawrence Barwick wrote:
>>
>> 2014/1/29 Ian Lawrence Barwick <barw...@gmail.com>:
>>>
>>> 2014-01-29 Andrew Dunstan <and...@dunslane.net>:
>>>>
>>>> On 01/28/2014 05:55 AM, Ian Lawrence Barwick wrote:
>>>>>
>>>>>
>>>>> Hi Payal
>>>>>
>>>>> Many thanks for the review, and my apologies for not getting back to
>>>>> you earlier.
>>>>>
>>>>> Updated version of the patch attached with suggested corrections.
>>>>
>>>> On a very quick glance, I see that you have still not made adjustments
>>>> to
>>>> contrib/file_fdw to accommodate this new option. I don't see why this
>>>> COPY
>>>> option should be different in that respect.
>>>
>>> Hmm, that idea seems to have escaped me completely. I'll get onto it
>>> forthwith.
>>
>> Striking while the keyboard is hot... version with contrib/file_fdw
>> modifications
>> attached.
>>
>>

> I have reviewed this. Generally it's good, but the author has made a
> significant error - the idea is not to force a quoted empty string to null,
> but to force a quoted null string to null, whatever the null string might
> be. The default case has these the same, but if you specify a non-empty null
> string they aren't.

The author slaps himself on the forehead while regretting he was temporally
constricted when dealing with the patch and never thought to look beyond
the immediate use case.

Thanks for the update, much appreciated.

> That difference actually made the file_fdw regression results plain wrong,
> in my view, in that they expected a quoted empty string to be turned to null
> even when the null string was something else.
>
> I've adjusted this and the docs and propose to apply the attached patch in
> the next day or two unless there are any objections.

Unless I'm overlooking something, output from "SELECT * FROM text_csv;"
in 'output/file_fdw.source' still needs updating?


Regards

Ian Barwick
diff --git a/contrib/file_fdw/data/text.csv b/contrib/file_fdw/data/text.csv
new file mode 100644
index ed348a9..f55d9cf
*** a/contrib/file_fdw/data/text.csv
--- b/contrib/file_fdw/data/text.csv
***************
*** 1,4 ****
! AAA,aaa
! XYZ,xyz
! NULL,NULL
! ABC,abc
--- 1,5 ----
! AAA,aaa,123,""
! XYZ,xyz,"",321
! NULL,NULL,NULL,NULL
! NULL,NULL,"NULL",NULL
! ABC,abc,"",""
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
new file mode 100644
index 5639f4d..7fb1dbc
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
*************** struct FileFdwOption
*** 48,56 ****
  
  /*
   * Valid options for file_fdw.
!  * These options are based on the options for COPY FROM command.
!  * But note that force_not_null is handled as a boolean option attached to
!  * each column, not as a table option.
   *
   * Note: If you are adding new option for user mapping, you need to modify
   * fileGetOptions(), which currently doesn't bother to look at user mappings.
--- 48,56 ----
  
  /*
   * Valid options for file_fdw.
!  * These options are based on the options for the COPY FROM command.
!  * But note that force_not_null and force_null are handled as boolean options
!  * attached to a column, not as table options.
   *
   * Note: If you are adding new option for user mapping, you need to modify
   * fileGetOptions(), which currently doesn't bother to look at user mappings.
*************** static const struct FileFdwOption valid_
*** 69,75 ****
  	{"null", ForeignTableRelationId},
  	{"encoding", ForeignTableRelationId},
  	{"force_not_null", AttributeRelationId},
! 
  	/*
  	 * force_quote is not supported by file_fdw because it's for COPY TO.
  	 */
--- 69,75 ----
  	{"null", ForeignTableRelationId},
  	{"encoding", ForeignTableRelationId},
  	{"force_not_null", AttributeRelationId},
! 	{"force_null", AttributeRelationId},
  	/*
  	 * force_quote is not supported by file_fdw because it's for COPY TO.
  	 */
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 187,192 ****
--- 187,193 ----
  	Oid			catalog = PG_GETARG_OID(1);
  	char	   *filename = NULL;
  	DefElem    *force_not_null = NULL;
+ 	DefElem    *force_null = NULL;
  	List	   *other_options = NIL;
  	ListCell   *cell;
  
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 243,252 ****
  		}
  
  		/*
! 		 * Separate out filename and force_not_null, since ProcessCopyOptions
! 		 * won't accept them.  (force_not_null only comes in a boolean
! 		 * per-column flavor here.)
  		 */
  		if (strcmp(def->defname, "filename") == 0)
  		{
  			if (filename)
--- 244,253 ----
  		}
  
  		/*
! 		 * Separate out filename and column-specific options, since
! 		 * ProcessCopyOptions won't accept them.
  		 */
+ 
  		if (strcmp(def->defname, "filename") == 0)
  		{
  			if (filename)
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 255,270 ****
  						 errmsg("conflicting or redundant options")));
  			filename = defGetString(def);
  		}
  		else if (strcmp(def->defname, "force_not_null") == 0)
  		{
  			if (force_not_null)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
! 						 errmsg("conflicting or redundant options")));
  			force_not_null = def;
  			/* Don't care what the value is, as long as it's a legal boolean */
  			(void) defGetBoolean(def);
  		}
  		else
  			other_options = lappend(other_options, def);
  	}
--- 256,297 ----
  						 errmsg("conflicting or redundant options")));
  			filename = defGetString(def);
  		}
+ 		/*
+ 		 * force_not_null is a boolean option; after validation we can discard
+ 		 * it - it will be retrieved later in get_file_fdw_attribute_options()
+ 		 */
  		else if (strcmp(def->defname, "force_not_null") == 0)
  		{
  			if (force_not_null)
  				ereport(ERROR,
  						(errcode(ERRCODE_SYNTAX_ERROR),
! 						 errmsg("conflicting or redundant options"),
! 						 errhint("option \"force_not_null\" supplied more than once for a column")));
! 			if(force_null)
! 				ereport(ERROR,
! 						(errcode(ERRCODE_SYNTAX_ERROR),
! 						 errmsg("conflicting or redundant options"),
! 						 errhint("option \"force_not_null\" cannot be used together with \"force_null\"")));
  			force_not_null = def;
  			/* Don't care what the value is, as long as it's a legal boolean */
  			(void) defGetBoolean(def);
  		}
+ 		/* See comments for force_not_null above */
+ 		else if (strcmp(def->defname, "force_null") == 0)
+ 		{
+ 			if (force_null)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_SYNTAX_ERROR),
+ 						 errmsg("conflicting or redundant options"),
+ 						 errhint("option \"force_null\" supplied more than once for a column")));
+ 			if(force_not_null)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_SYNTAX_ERROR),
+ 						 errmsg("conflicting or redundant options"),
+ 						 errhint("option \"force_null\" cannot be used together with \"force_not_null\"")));
+ 			force_null = def;
+ 			(void) defGetBoolean(def);
+ 		}
  		else
  			other_options = lappend(other_options, def);
  	}
*************** fileGetOptions(Oid foreigntableid,
*** 369,376 ****
   * Retrieve per-column generic options from pg_attribute and construct a list
   * of DefElems representing them.
   *
!  * At the moment we only have "force_not_null", which should be combined into
!  * a single DefElem listing all such columns, since that's what COPY expects.
   */
  static List *
  get_file_fdw_attribute_options(Oid relid)
--- 396,404 ----
   * Retrieve per-column generic options from pg_attribute and construct a list
   * of DefElems representing them.
   *
!  * At the moment we only have "force_not_null", and "force_null",
!  * which should each be combined into a single DefElem listing all such
!  * columns, since that's what COPY expects.
   */
  static List *
  get_file_fdw_attribute_options(Oid relid)
*************** get_file_fdw_attribute_options(Oid relid
*** 380,385 ****
--- 408,416 ----
  	AttrNumber	natts;
  	AttrNumber	attnum;
  	List	   *fnncolumns = NIL;
+ 	List	   *fncolumns = NIL;
+ 
+ 	List *options = NIL;
  
  	rel = heap_open(relid, AccessShareLock);
  	tupleDesc = RelationGetDescr(rel);
*************** get_file_fdw_attribute_options(Oid relid
*** 410,426 ****
  					fnncolumns = lappend(fnncolumns, makeString(attname));
  				}
  			}
  			/* maybe in future handle other options here */
  		}
  	}
  
  	heap_close(rel, AccessShareLock);
  
! 	/* Return DefElem only when some column(s) have force_not_null */
  	if (fnncolumns != NIL)
! 		return list_make1(makeDefElem("force_not_null", (Node *) fnncolumns));
! 	else
! 		return NIL;
  }
  
  /*
--- 441,469 ----
  					fnncolumns = lappend(fnncolumns, makeString(attname));
  				}
  			}
+ 			else if (strcmp(def->defname, "force_null") == 0)
+ 			{
+ 				if (defGetBoolean(def))
+ 				{
+ 					char	   *attname = pstrdup(NameStr(attr->attname));
+ 
+ 					fncolumns = lappend(fncolumns, makeString(attname));
+ 				}
+ 			}
  			/* maybe in future handle other options here */
  		}
  	}
  
  	heap_close(rel, AccessShareLock);
  
! 	/* Return DefElem only when some column(s) have force_not_null / force_null options set */
  	if (fnncolumns != NIL)
! 		options = lappend(options, makeDefElem("force_not_null", (Node *) fnncolumns));
! 
! 	if (fncolumns != NIL)
! 		options = lappend(options,makeDefElem("force_null", (Node *) fncolumns));
! 
! 	return options;
  }
  
  /*
diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
new file mode 100644
index f7fd28d..0c278aa
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** OPTIONS (format 'csv', filename '@abs_sr
*** 81,91 ****
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
      word1 text OPTIONS (force_not_null 'true'),
!     word2 text OPTIONS (force_not_null 'off')
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
  SELECT * FROM text_csv; -- ERROR
  ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
  SELECT * FROM text_csv;
  
  -- force_not_null is not allowed to be specified at any foreign object level:
--- 81,94 ----
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
      word1 text OPTIONS (force_not_null 'true'),
!     word2 text OPTIONS (force_not_null 'off'),
!     word3 text OPTIONS (force_null 'true'),
!     word4 text OPTIONS (force_null 'off')
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
  SELECT * FROM text_csv; -- ERROR
  ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
+ \pset null _null_
  SELECT * FROM text_csv;
  
  -- force_not_null is not allowed to be specified at any foreign object level:
*************** ALTER SERVER file_server OPTIONS (ADD fo
*** 94,99 ****
--- 97,114 ----
  CREATE USER MAPPING FOR public SERVER file_server OPTIONS (force_not_null '*'); -- ERROR
  CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_not_null '*'); -- ERROR
  
+ -- force_not_null cannot be specified together with force_null
+ ALTER FOREIGN TABLE text_csv ALTER COLUMN word1 OPTIONS (force_null 'true'); --ERROR
+ 
+ -- force_null is not allowed to be specified at any foreign object level:
+ ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_null '*'); -- ERROR
+ ALTER SERVER file_server OPTIONS (ADD force_null '*'); -- ERROR
+ CREATE USER MAPPING FOR public SERVER file_server OPTIONS (force_null '*'); -- ERROR
+ CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_null '*'); -- ERROR
+ 
+ -- force_null cannot be specified together with force_not_null
+ ALTER FOREIGN TABLE text_csv ALTER COLUMN word3 OPTIONS (force_not_null 'true'); --ERROR
+ 
  -- basic query tests
  SELECT * FROM agg_text WHERE b > 10.0 ORDER BY a;
  SELECT * FROM agg_csv ORDER BY a;
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
new file mode 100644
index 4f90bae..2bec160
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** OPTIONS (format 'csv', filename '@abs_sr
*** 96,115 ****
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
      word1 text OPTIONS (force_not_null 'true'),
!     word2 text OPTIONS (force_not_null 'off')
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
  SELECT * FROM text_csv; -- ERROR
  ERROR:  COPY force not null available only in CSV mode
  ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
  SELECT * FROM text_csv;
!  word1 | word2 
! -------+-------
!  AAA   | aaa
!  XYZ   | xyz
!  NULL  | 
!  ABC   | abc
! (4 rows)
  
  -- force_not_null is not allowed to be specified at any foreign object level:
  ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_not_null '*'); -- ERROR
--- 96,119 ----
  -- per-column options tests
  CREATE FOREIGN TABLE text_csv (
      word1 text OPTIONS (force_not_null 'true'),
!     word2 text OPTIONS (force_not_null 'off'),
!     word3 text OPTIONS (force_null 'true'),
!     word4 text OPTIONS (force_null 'off')
  ) SERVER file_server
  OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
  SELECT * FROM text_csv; -- ERROR
  ERROR:  COPY force not null available only in CSV mode
  ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
+ \pset null _null_
  SELECT * FROM text_csv;
!  word1 | word2  | word3  | word4  
! -------+--------+--------+--------
!  AAA   | aaa    | 123    | 
!  XYZ   | xyz    |        | 321
!  NULL  | _null_ | _null_ | _null_
!  NULL  | _null_ | _null_ | _null_
!  ABC   | abc    |        | 
! (5 rows)
  
  -- force_not_null is not allowed to be specified at any foreign object level:
  ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_not_null '*'); -- ERROR
*************** HINT:  There are no valid options in thi
*** 124,129 ****
--- 128,154 ----
  CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_not_null '*'); -- ERROR
  ERROR:  invalid option "force_not_null"
  HINT:  Valid options in this context are: filename, format, header, delimiter, quote, escape, null, encoding
+ -- force_not_null cannot be specified together with force_null
+ ALTER FOREIGN TABLE text_csv ALTER COLUMN word1 OPTIONS (force_null 'true'); --ERROR
+ ERROR:  conflicting or redundant options
+ HINT:  option "force_null" cannot be used together with "force_not_null"
+ -- force_null is not allowed to be specified at any foreign object level:
+ ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_null '*'); -- ERROR
+ ERROR:  invalid option "force_null"
+ HINT:  There are no valid options in this context.
+ ALTER SERVER file_server OPTIONS (ADD force_null '*'); -- ERROR
+ ERROR:  invalid option "force_null"
+ HINT:  There are no valid options in this context.
+ CREATE USER MAPPING FOR public SERVER file_server OPTIONS (force_null '*'); -- ERROR
+ ERROR:  invalid option "force_null"
+ HINT:  There are no valid options in this context.
+ CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (force_null '*'); -- ERROR
+ ERROR:  invalid option "force_null"
+ HINT:  Valid options in this context are: filename, format, header, delimiter, quote, escape, null, encoding
+ -- force_null cannot be specified together with force_not_null
+ ALTER FOREIGN TABLE text_csv ALTER COLUMN word3 OPTIONS (force_not_null 'true'); --ERROR
+ ERROR:  conflicting or redundant options
+ HINT:  option "force_not_null" cannot be used together with "force_null"
  -- basic query tests
  SELECT * FROM agg_text WHERE b > 10.0 ORDER BY a;
    a  |   b    
diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml
new file mode 100644
index 9385b26..d3b39aa
*** a/doc/src/sgml/file-fdw.sgml
--- b/doc/src/sgml/file-fdw.sgml
***************
*** 112,122 ****
   </variablelist>
  
   <para>
!   Note that while <command>COPY</> allows options such as OIDS and HEADER 
    to be specified without a corresponding value, the foreign data wrapper
!   syntax requires a value to be present in all cases.  To activate 
    <command>COPY</> options normally supplied without a value, you can
!   instead pass the value TRUE. 
   </para>
  
   <para>
--- 112,122 ----
   </variablelist>
  
   <para>
!   Note that while <command>COPY</> allows options such as OIDS and HEADER
    to be specified without a corresponding value, the foreign data wrapper
!   syntax requires a value to be present in all cases.  To activate
    <command>COPY</> options normally supplied without a value, you can
!   instead pass the value TRUE.
   </para>
  
   <para>
***************
*** 139,144 ****
--- 139,159 ----
      </para>
     </listitem>
    </varlistentry>
+ 
+   <varlistentry>
+    <term><literal>force_null</literal></term>
+ 
+    <listitem>
+     <para>
+      This is a Boolean option.  If true, it specifies that values of the
+      column which match the null string are returned as <literal>NULL</>
+      even if the value is quoted. Without this option, only unquoted
+      values matching the null string are returned as <literal>NULL</>.
+      This has the same effect  as listing the column in
+      <command>COPY</>'s <literal>FORCE_NULL</literal> option.
+     </para>
+    </listitem>
+   </varlistentry>
  
   </variablelist>
  
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
new file mode 100644
index 99f246a..5be3514
*** a/doc/src/sgml/ref/copy.sgml
--- b/doc/src/sgml/ref/copy.sgml
*************** COPY { <replaceable class="parameter">ta
*** 42,47 ****
--- 42,48 ----
      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> [, ...] )
      ENCODING '<replaceable class="parameter">encoding_name</replaceable>'
  </synopsis>
   </refsynopsisdiv>
*************** COPY { <replaceable class="parameter">ta
*** 329,334 ****
--- 330,349 ----
     </varlistentry>
  
     <varlistentry>
+     <term><literal>FORCE_NULL</></term>
+     <listitem>
+      <para>
+       Match the specified columns' values against the null string, even
+       if it has been quoted, and if a match is found set the value to
+       <literal>NULL</>. In the default case where the null string is empty,
+       this converts a quoted empty string into NULL.
+       This option is allowed only in <command>COPY FROM</>, and only when
+       using <literal>CSV</> format.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
      <term><literal>ENCODING</></term>
      <listitem>
       <para>
*************** COPY <replaceable class="parameter">coun
*** 637,643 ****
      string, while an empty string data value is written with double quotes
      (<literal>""</>). Reading values follows similar rules. You can
      use <literal>FORCE_NOT_NULL</> to prevent <literal>NULL</> input
!     comparisons for specific columns.
     </para>
  
     <para>
--- 652,660 ----
      string, while an empty string data value is written with double quotes
      (<literal>""</>). Reading values follows similar rules. You can
      use <literal>FORCE_NOT_NULL</> to prevent <literal>NULL</> input
!     comparisons for specific columns. You can also use
!     <literal>FORCE_NULL</> to convert quoted null string data values to
!     <literal>NULL</>.
     </para>
  
     <para>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
new file mode 100644
index 7c4039c..70ee7e5
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*************** typedef struct CopyStateData
*** 125,130 ****
--- 125,132 ----
  	bool	   *force_quote_flags;		/* per-column CSV FQ flags */
  	List	   *force_notnull;	/* list of column names */
  	bool	   *force_notnull_flags;	/* per-column CSV FNN flags */
+ 	List	   *force_null;	/* list of column names */
+ 	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) */
  	bool	   *convert_select_flags;	/* per-column CSV/TEXT CS flags */
*************** ProcessCopyOptions(CopyState cstate,
*** 1019,1024 ****
--- 1021,1040 ----
  						 errmsg("argument to option \"%s\" must be a list of column names",
  								defel->defname)));
  		}
+ 		else if (strcmp(defel->defname, "force_null") == 0)
+ 		{
+ 			if (cstate->force_null)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_SYNTAX_ERROR),
+ 						 errmsg("conflicting or redundant options")));
+ 			if (defel->arg && IsA(defel->arg, List))
+ 				cstate->force_null = (List *) defel->arg;
+ 			else
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ 						 errmsg("argument to option \"%s\" must be a list of column names",
+ 								defel->defname)));
+ 		}
  		else if (strcmp(defel->defname, "convert_selectively") == 0)
  		{
  			/*
*************** ProcessCopyOptions(CopyState cstate,
*** 1178,1183 ****
--- 1194,1210 ----
  				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
  			  errmsg("COPY force not null only available using COPY FROM")));
  
+ 	/* Check force_null */
+ 	if (!cstate->csv_mode && cstate->force_null != NIL)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 				 errmsg("COPY force null available only in CSV mode")));
+ 
+ 	if (cstate->force_null != NIL && !is_from)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 			  errmsg("COPY force null only available using COPY FROM")));
+ 
  	/* Don't allow the delimiter to appear in the null string. */
  	if (strchr(cstate->null_print, cstate->delim[0]) != NULL)
  		ereport(ERROR,
*************** BeginCopy(bool is_from,
*** 1385,1390 ****
--- 1412,1439 ----
  		}
  	}
  
+ 	/* Convert FORCE NULL name list to per-column flags, check validity */
+ 	cstate->force_null_flags = (bool *) palloc0(num_phys_attrs * sizeof(bool));
+ 	if (cstate->force_null)
+ 	{
+ 		List	   *attnums;
+ 		ListCell   *cur;
+ 
+ 		attnums = CopyGetAttnums(tupDesc, cstate->rel, cstate->force_null);
+ 
+ 		foreach(cur, attnums)
+ 		{
+ 			int			attnum = lfirst_int(cur);
+ 
+ 			if (!list_member_int(cstate->attnumlist, attnum))
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+ 				errmsg("FORCE NULL column \"%s\" not referenced by COPY",
+ 					   NameStr(tupDesc->attrs[attnum - 1]->attname))));
+ 			cstate->force_null_flags[attnum - 1] = true;
+ 		}
+ 	}
+ 
  	/* Convert convert_selectively name list to per-column flags */
  	if (cstate->convert_selectively)
  	{
*************** NextCopyFrom(CopyState cstate, ExprConte
*** 2810,2820 ****
  				continue;
  			}
  
! 			if (cstate->csv_mode && string == NULL &&
! 				cstate->force_notnull_flags[m])
  			{
! 				/* Go ahead and read the NULL string */
! 				string = cstate->null_print;
  			}
  
  			cstate->cur_attname = NameStr(attr[m]->attname);
--- 2859,2886 ----
  				continue;
  			}
  
! 			if (cstate->csv_mode)
  			{
! 				if(string == NULL &&
! 				   cstate->force_notnull_flags[m])
! 				{
! 					/*
! 					 * FORCE_NOT_NULL option is set and column is NULL -
! 					 * convert it to the NULL string.
! 					 */
! 					string = cstate->null_print;
! 				}
! 				else if(string != NULL && cstate->force_null_flags[m]
! 						&& strcmp(string,cstate->null_print) == 0 )
! 				{
! 					/*
! 					 * FORCE_NULL option is set and column matches the NULL string.
! 					 * It must have been quoted, or otherwise the string would already
! 					 * have been set to NULL.
! 					 * Convert it to NULL as specified.
! 					 */
! 					string = NULL;
! 				}
  			}
  
  			cstate->cur_attname = NameStr(attr[m]->attname);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index 81169a4..e3060a4
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** copy_opt_item:
*** 2548,2553 ****
--- 2548,2557 ----
  				{
  					$$ = makeDefElem("force_not_null", (Node *)$4);
  				}
+ 			| FORCE NULL_P columnList
+ 				{
+ 					$$ = makeDefElem("force_null", (Node *)$3);
+ 				}
  			| ENCODING Sconst
  				{
  					$$ = makeDefElem("encoding", (Node *)makeString($2));
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
new file mode 100644
index 34fa131..de99108
*** a/src/test/regress/expected/copy2.out
--- b/src/test/regress/expected/copy2.out
*************** SELECT * FROM vistest;
*** 382,387 ****
--- 382,435 ----
   e
  (2 rows)
  
+ -- Test FORCE_NOT_NULL and FORCE_NULL options
+ -- should succeed with "b" set to an empty string and "c" set to NULL
+ CREATE TEMP 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) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c));
+ COMMIT;
+ SELECT b, c FROM forcetest WHERE a = 1;
+  b |  c   
+ ---+------
+    | NULL
+ (1 row)
+ 
+ -- 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(b), FORCE_NULL(c));
+ COMMIT;
+ SELECT b, c FROM forcetest WHERE a = 2;
+  b |  c   
+ ---+------
+    | NULL
+ (1 row)
+ 
+ -- should fail with not-null constraint violiaton
+ BEGIN;
+ COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b), FORCE_NOT_NULL(c));
+ ERROR:  null value in column "b" violates not-null constraint
+ DETAIL:  Failing row contains (3, null, , null, null).
+ CONTEXT:  COPY forcetest, line 1: "3,,"""
+ ROLLBACK;
+ -- should fail with "not referenced by COPY" error
+ BEGIN;
+ COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b));
+ ERROR:  FORCE NOT NULL column "b" not referenced by COPY
+ ROLLBACK;
+ -- should fail with "not referenced by COPY" error
+ BEGIN;
+ COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
+ ERROR:  FORCE NULL column "b" not referenced by COPY
+ ROLLBACK;
+ \pset null ''
+ DROP TABLE forcetest;
  DROP TABLE vistest;
  DROP FUNCTION truncate_in_subxact();
  DROP TABLE x, y;
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
new file mode 100644
index c46128b..b417cf7
*** a/src/test/regress/sql/copy2.sql
--- b/src/test/regress/sql/copy2.sql
*************** e
*** 270,275 ****
--- 270,314 ----
  SELECT * FROM vistest;
  COMMIT;
  SELECT * FROM vistest;
+ -- Test FORCE_NOT_NULL and FORCE_NULL options
+ -- should succeed with "b" set to an empty string and "c" set to NULL
+ CREATE TEMP 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) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b), FORCE_NULL(c));
+ 1,,""
+ \.
+ COMMIT;
+ SELECT b, c FROM forcetest WHERE a = 1;
+ -- 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(b), FORCE_NULL(c));
+ 2,,""
+ \.
+ COMMIT;
+ SELECT b, c FROM forcetest WHERE a = 2;
+ -- should fail with not-null constraint violiaton
+ BEGIN;
+ COPY forcetest (a, b, c) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b), FORCE_NOT_NULL(c));
+ 3,,""
+ \.
+ ROLLBACK;
+ -- should fail with "not referenced by COPY" error
+ BEGIN;
+ COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NOT_NULL(b));
+ ROLLBACK;
+ -- should fail with "not referenced by COPY" error
+ BEGIN;
+ COPY forcetest (d, e) FROM STDIN WITH (FORMAT csv, FORCE_NULL(b));
+ ROLLBACK;
+ \pset null ''
+ DROP TABLE forcetest;
  DROP TABLE vistest;
  DROP FUNCTION truncate_in_subxact();
  DROP TABLE x, y;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to