not null validation option in contrib/file_fdw

Started by Etsuro Fujitaalmost 14 years ago6 messages

fujita.etsuro@lab.ntt.co.jp

almost 14 years ago

1 attachment(s)

I updated the patch added to CF 2012-Next [1]https://commitfest.postgresql.org/action/patch_view?id=822. Attached is the updated
version of the patch.

For the discussion in [2]http://archives.postgresql.org/message-id/1038.1331738954@sss.pgh.pa.us, I've introduced a new generic option, validate
for file_fdw foreign tables, which specifies if file_fdw verifies that
tuples meet NOT NULL constraints. The default value for the option is
'false', and if the value is set to 'true', then file_fdw verifies NOT NULL
constraints. For example, a user can issue the following to let file_fdw
verify the constraint of the msg column.

CREATE FOREIGN TABLE ft (id INTEGER, msg TEXT NOT NULL)
SERVER fs
OPTIONS (filename '/path/to/file', format 'csv',
delimiter ',', validate 'true');

At the SELECT time, ABORT is issued when a NOT NULL violation error has been
found, like COPY FROM. Once the validation is done successfully using e.g.
SELECT COUNT(*), the user can set the option to 'false' using ALTER FOREIGN
TABLE. I think this option is needed for flat files due to their lack of
ability to check such a constraint.

(I added NOT NULL checking to ileIterateForeignScan(), but not to
file_acquire_sample_rows(). Should we do that at
file_acquire_sample_rows()? I think it is good to just recommend that users
do ANALYZE a foreign table after the validation.)

For the discussion in [3]http://archives.postgresql.org/pgsql-hackers/2012-03/msg00809.php, I've added a new external function
ExecNotNullCheck() and call it from fileIterateForeignScan.

Any comments are welcome.

Best regards,
Etsuro Fujita

[1]: https://commitfest.postgresql.org/action/patch_view?id=822
[2]: http://archives.postgresql.org/message-id/1038.1331738954@sss.pgh.pa.us
[3]: http://archives.postgresql.org/pgsql-hackers/2012-03/msg00809.php

Attachments:

file_fdw_notnull_v2.patchapplication/octet-stream; name=file_fdw_notnull_v2.patchDownload

diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 2d36a72..f796a54 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -71,6 +71,9 @@ static const struct FileFdwOption valid_options[] = {
 	 * force_quote is not supported by file_fdw because it's for COPY TO.
 	 */
 
+	/* Validation options */
+	{"validate", ForeignTableRelationId},
+
 	/* Sentinel */
 	{NULL, InvalidOid}
 };
@@ -93,6 +96,7 @@ typedef struct FileFdwExecutionState
 {
 	char	   *filename;		/* file to read */
 	List	   *options;		/* merged COPY options, excluding filename */
+	bool		validate;		/* whether to validate NOT NULL constraint */
 	CopyState	cstate;			/* state of reading file */
 } FileFdwExecutionState;
 
@@ -134,7 +138,9 @@ static bool fileAnalyzeForeignTable(Relation relation,
  */
 static bool is_valid_option(const char *option, Oid context);
 static void fileGetOptions(Oid foreigntableid,
-			   char **filename, List **other_options);
+						   char **filename,
+						   List **other_options,
+						   bool *validate);
 static List *get_file_fdw_attribute_options(Oid relid);
 static void estimate_size(PlannerInfo *root, RelOptInfo *baserel,
 			  FileFdwPlanState *fdw_private);
@@ -180,6 +186,7 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 	List	   *options_list = untransformRelOptions(PG_GETARG_DATUM(0));
 	Oid			catalog = PG_GETARG_OID(1);
 	char	   *filename = NULL;
+	DefElem	   *validate = NULL;
 	DefElem	   *force_not_null = NULL;
 	List	   *other_options = NIL;
 	ListCell   *cell;
@@ -247,6 +254,16 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 						 errmsg("conflicting or redundant options")));
 			filename = defGetString(def);
 		}
+		else if (strcmp(def->defname, "validate") == 0)
+		{
+			if (validate)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
+			validate = def;
+			/* Don't care what the value is, as long as it's a legal boolean */
+			(void) defGetBoolean(def);
+		}
 		else if (strcmp(def->defname, "force_not_null") == 0)
 		{
 			if (force_not_null)
@@ -301,8 +318,8 @@ is_valid_option(const char *option, Oid context)
  * it must not appear in the options list passed to the core COPY code.
  */
 static void
-fileGetOptions(Oid foreigntableid,
-			   char **filename, List **other_options)
+fileGetOptions(Oid foreigntableid, char **filename,
+			   List **other_options, bool *validate)
 {
 	ForeignTable *table;
 	ForeignServer *server;
@@ -333,6 +350,7 @@ fileGetOptions(Oid foreigntableid,
 	 * Separate out the filename.
 	 */
 	*filename = NULL;
+	*validate = false;
 	prev = NULL;
 	foreach(lc, options)
 	{
@@ -342,7 +360,11 @@ fileGetOptions(Oid foreigntableid,
 		{
 			*filename = defGetString(def);
 			options = list_delete_cell(options, lc, prev);
-			break;
+		}
+		if (strcmp(def->defname, "validate") == 0)
+		{
+			*validate = defGetBoolean(def);
+			options = list_delete_cell(options, lc, prev);
 		}
 		prev = lc;
 	}
@@ -425,6 +447,7 @@ fileGetForeignRelSize(PlannerInfo *root,
 					  Oid foreigntableid)
 {
 	FileFdwPlanState *fdw_private;
+	bool validate;
 
 	/*
 	 * Fetch options.  We only need filename at this point, but we might
@@ -432,7 +455,9 @@ fileGetForeignRelSize(PlannerInfo *root,
 	 */
 	fdw_private = (FileFdwPlanState *) palloc(sizeof(FileFdwPlanState));
 	fileGetOptions(foreigntableid,
-				   &fdw_private->filename, &fdw_private->options);
+				   &fdw_private->filename,
+				   &fdw_private->options,
+				   &validate);
 	baserel->fdw_private = (void *) fdw_private;
 
 	/* Estimate relation size */
@@ -517,10 +542,11 @@ fileExplainForeignScan(ForeignScanState *node, ExplainState *es)
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate;
 
 	/* Fetch options --- we only need filename at this point */
 	fileGetOptions(RelationGetRelid(node->ss.ss_currentRelation),
-				   &filename, &options);
+				   &filename, &options, &validate);
 
 	ExplainPropertyText("Foreign File", filename, es);
 
@@ -544,6 +570,7 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate;
 	CopyState	cstate;
 	FileFdwExecutionState *festate;
 
@@ -555,7 +582,7 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 
 	/* Fetch options of foreign table */
 	fileGetOptions(RelationGetRelid(node->ss.ss_currentRelation),
-				   &filename, &options);
+				   &filename, &options, &validate);
 
 	/*
 	 * Create CopyState from FDW options.  We always acquire all columns, so
@@ -573,6 +600,7 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 	festate = (FileFdwExecutionState *) palloc(sizeof(FileFdwExecutionState));
 	festate->filename = filename;
 	festate->options = options;
+	festate->validate = validate;
 	festate->cstate = cstate;
 
 	node->fdw_state = (void *) festate;
@@ -588,6 +616,8 @@ fileIterateForeignScan(ForeignScanState *node)
 {
 	FileFdwExecutionState *festate = (FileFdwExecutionState *) node->fdw_state;
 	TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
+	Relation	rel = node->ss.ss_currentRelation;
+	TupleConstr *constr = rel->rd_att->constr;
 	bool		found;
 	ErrorContextCallback errcontext;
 
@@ -614,7 +644,11 @@ fileIterateForeignScan(ForeignScanState *node)
 						 slot->tts_values, slot->tts_isnull,
 						 NULL);
 	if (found)
+	{
 		ExecStoreVirtualTuple(slot);
+		if (constr && constr->has_not_null && festate->validate)
+			ExecNotNullCheck(rel, slot);
+	}
 
 	/* Remove error callback. */
 	error_context_stack = errcontext.previous;
@@ -664,10 +698,11 @@ fileAnalyzeForeignTable(Relation relation,
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate;
 	struct stat	stat_buf;
 
 	/* Fetch options of foreign table */
-	fileGetOptions(RelationGetRelid(relation), &filename, &options);
+	fileGetOptions(RelationGetRelid(relation), &filename, &options, &validate);
 
 	/*
 	 * Get size of the file.  (XXX if we fail here, would it be better to
@@ -831,6 +866,7 @@ file_acquire_sample_rows(Relation onerel, int elevel,
 	bool		found;
 	char	   *filename;
 	List	   *options;
+	bool		validate;
 	CopyState	cstate;
 	ErrorContextCallback errcontext;
 	MemoryContext oldcontext = CurrentMemoryContext;
@@ -844,7 +880,7 @@ file_acquire_sample_rows(Relation onerel, int elevel,
 	nulls = (bool *) palloc(tupDesc->natts * sizeof(bool));
 
 	/* Fetch options of foreign table */
-	fileGetOptions(RelationGetRelid(onerel), &filename, &options);
+	fileGetOptions(RelationGetRelid(onerel), &filename, &options, &validate);
 
 	/*
 	 * Create CopyState from FDW options.
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 84f0750..017f193 100644
--- a/contrib/file_fdw/output/file_fdw.source
+++ b/contrib/file_fdw/output/file_fdw.source
@@ -123,7 +123,7 @@ ERROR:  invalid option "force_not_null"
 HINT:  Valid options in this context are: 
 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
+HINT:  Valid options in this context are: filename, format, header, delimiter, quote, escape, null, encoding, validate
 -- 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
index 4c7f7e8..d6b474b 100644
--- a/doc/src/sgml/file-fdw.sgml
+++ b/doc/src/sgml/file-fdw.sgml
@@ -108,6 +108,16 @@
    </listitem>
   </varlistentry>
 
+  <varlistentry>
+   <term><literal>validate</literal></term>
+
+   <listitem>
+    <para>
+     Specifies whether the <literal>NOT NULL</literal> constraint is validated.
+    </para>
+   </listitem>
+  </varlistentry>
+
  </variablelist>
 
  <para>
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index fbb36fa..5417eea 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -80,7 +80,7 @@ static void ExecutePlan(EState *estate, PlanState *planstate,
 static bool ExecCheckRTEPerms(RangeTblEntry *rte);
 static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt);
 static char *ExecBuildSlotValueDescription(TupleTableSlot *slot,
-										   int maxfieldlen);
+					   int maxfieldlen);
 static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
 				  Plan *planTree);
 
@@ -1438,6 +1438,28 @@ ExecutePlan(EState *estate,
 
 
 /*
+ * ExecNotNullCheck --- check that tuple meets NOT NULL constraints
+ */
+void
+ExecNotNullCheck(Relation rel, TupleTableSlot *slot)
+{
+	int			natts = rel->rd_att->natts;
+	int			attrChk;
+
+	for (attrChk = 1; attrChk <= natts; attrChk++)
+	{
+		if (rel->rd_att->attrs[attrChk - 1]->attnotnull &&
+			slot_attisnull(slot, attrChk))
+			ereport(ERROR,
+					(errcode(ERRCODE_NOT_NULL_VIOLATION),
+					 errmsg("null value in column \"%s\" violates not-null constraint",
+							NameStr(rel->rd_att->attrs[attrChk - 1]->attname)),
+					 errdetail("Failing row contains %s.",
+							   ExecBuildSlotValueDescription(slot, 64))));
+	}
+}
+
+/*
  * ExecRelCheck --- check that tuple meets constraints for result relation
  */
 static const char *
@@ -1509,22 +1531,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 	Assert(constr);
 
 	if (constr->has_not_null)
-	{
-		int			natts = rel->rd_att->natts;
-		int			attrChk;
-
-		for (attrChk = 1; attrChk <= natts; attrChk++)
-		{
-			if (rel->rd_att->attrs[attrChk - 1]->attnotnull &&
-				slot_attisnull(slot, attrChk))
-				ereport(ERROR,
-						(errcode(ERRCODE_NOT_NULL_VIOLATION),
-						 errmsg("null value in column \"%s\" violates not-null constraint",
-						NameStr(rel->rd_att->attrs[attrChk - 1]->attname)),
-						 errdetail("Failing row contains %s.",
-								   ExecBuildSlotValueDescription(slot, 64))));
-		}
-	}
+		ExecNotNullCheck(rel, slot);
 
 	if (constr->num_check > 0)
 	{
@@ -1548,7 +1555,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
  * here since heap field values could be very long, whereas index entries
  * typically aren't so wide.
  */
-static char *
+char *
 ExecBuildSlotValueDescription(TupleTableSlot *slot, int maxfieldlen)
 {
 	StringInfoData buf;
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f5503a5..5eda8ea 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -188,6 +188,7 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
 extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
+extern void ExecNotNullCheck(Relation rel, TupleTableSlot *slot);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
 				TupleTableSlot *slot, EState *estate);
 extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti);

Shigeru HANADA

shigeru.hanada@gmail.com

over 13 years ago

In reply to: Etsuro Fujita (#1)

Re: not null validation option in contrib/file_fdw

(2012/04/13 16:59), Etsuro Fujita wrote:

I updated the patch added to CF 2012-Next [1]. Attached is the updated
version of the patch.

I applied the patch and ran regression tests of file_fdw, and I got
SIGSEGV X-(

The failure occurs in fileGetOptions, and it is caused by
list_delete_cell used in foreach loop; ListCell points delete target has
been free-ed in list_delete_cell, but foreach accesses it to get next
element.

Some of backend functions which use list_delete_cell in loop use "for"
loop instead of foreach, and other functions exit the loop after calling
list_delete_cell. Since we can't stop searching non-COPY options until
meeting the end of the options list, we would need to choose former
("for" loop), or create another list which contains only valid COPY
options and return it via other_options parameter.

Regards,
--
Shigeru HANADA

Andrew Dunstan

andrew@dunslane.net

over 13 years ago

In reply to: Shigeru HANADA (#2)

Re: not null validation option in contrib/file_fdw

On 04/13/2012 07:21 AM, Shigeru HANADA wrote:

(2012/04/13 16:59), Etsuro Fujita wrote:

I updated the patch added to CF 2012-Next [1]. Attached is the updated
version of the patch.

I applied the patch and ran regression tests of file_fdw, and I got
SIGSEGV X-(

The failure occurs in fileGetOptions, and it is caused by
list_delete_cell used in foreach loop; ListCell points delete target has
been free-ed in list_delete_cell, but foreach accesses it to get next
element.

Some of backend functions which use list_delete_cell in loop use "for"
loop instead of foreach, and other functions exit the loop after calling
list_delete_cell. Since we can't stop searching non-COPY options until
meeting the end of the options list, we would need to choose former
("for" loop), or create another list which contains only valid COPY
options and return it via other_options parameter.

Yes, the code in fileGetOptions() appears to be bogus.

Also, "validate" is a terrible name for the option (and in the code)
IMNSHO. It's far too generic. "validate_not_null" or some such would
surely be better.

cheers

andrew

Etsuro Fujita

fujita.etsuro@lab.ntt.co.jp

over 13 years ago

In reply to: Andrew Dunstan (#3)

Re: not null validation option in contrib/file_fdw

Thank you for the review.

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Andrew Dunstan
Sent: Friday, April 13, 2012 9:16 PM
To: Shigeru HANADA
Cc: Etsuro Fujita; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in contrib/file_fdw

On 04/13/2012 07:21 AM, Shigeru HANADA wrote:

(2012/04/13 16:59), Etsuro Fujita wrote:

I updated the patch added to CF 2012-Next [1]. Attached is the
updated version of the patch.

I applied the patch and ran regression tests of file_fdw, and I got
SIGSEGV X-(

The failure occurs in fileGetOptions, and it is caused by
list_delete_cell used in foreach loop; ListCell points delete target
has been free-ed in list_delete_cell, but foreach accesses it to get
next element.

Some of backend functions which use list_delete_cell in loop use "for"
loop instead of foreach, and other functions exit the loop after
calling list_delete_cell. Since we can't stop searching non-COPY
options until meeting the end of the options list, we would need to
choose former ("for" loop), or create another list which contains only
valid COPY options and return it via other_options parameter.

Yes, the code in fileGetOptions() appears to be bogus.

Sorry, I will fix it.

Also, "validate" is a terrible name for the option (and in the code)

IMNSHO.

It's far too generic. "validate_not_null" or some such would surely be
better.

I thought it would be used for not only NOT NULL but also CHECK and foreign
key constraints. That is, when a user sets the option to 'true', file_fdw
verifies that each tuple meets all kinds of constraints. So, how about
"validate_data_file" or simply "validate_file"?

Best regards,
Etsuro Fujita

Show quoted text

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make
changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Etsuro Fujita

fujita.etsuro@lab.ntt.co.jp

over 13 years ago

In reply to: Etsuro Fujita (#4)

1 attachment(s)

Re: not null validation option in contrib/file_fdw

I updated the patch. Attached is an updated version of the patch.

Changes:
* fix a bug in fileGetOptions()
* rename the validation option and its code to "validate_data_file"
* clean up

Best regards,
Etsuro Fujita

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Etsuro Fujita
Sent: Monday, April 16, 2012 4:09 PM
To: 'Andrew Dunstan'; 'Shigeru HANADA'
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in contrib/file_fdw

Thank you for the review.

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Andrew
Dunstan
Sent: Friday, April 13, 2012 9:16 PM
To: Shigeru HANADA
Cc: Etsuro Fujita; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in contrib/file_fdw

On 04/13/2012 07:21 AM, Shigeru HANADA wrote:

(2012/04/13 16:59), Etsuro Fujita wrote:

I updated the patch added to CF 2012-Next [1]. Attached is the
updated version of the patch.

I applied the patch and ran regression tests of file_fdw, and I got
SIGSEGV X-(

The failure occurs in fileGetOptions, and it is caused by
list_delete_cell used in foreach loop; ListCell points delete target
has been free-ed in list_delete_cell, but foreach accesses it to get
next element.

Some of backend functions which use list_delete_cell in loop use "for"
loop instead of foreach, and other functions exit the loop after
calling list_delete_cell. Since we can't stop searching non-COPY
options until meeting the end of the options list, we would need to
choose former ("for" loop), or create another list which contains
only valid COPY options and return it via other_options parameter.

Yes, the code in fileGetOptions() appears to be bogus.

Sorry, I will fix it.

Also, "validate" is a terrible name for the option (and in the code)

IMNSHO.

It's far too generic. "validate_not_null" or some such would surely be
better.

I thought it would be used for not only NOT NULL but also CHECK and

foreign

Show quoted text

key constraints. That is, when a user sets the option to 'true', file_fdw
verifies that each tuple meets all kinds of constraints. So, how about
"validate_data_file" or simply "validate_file"?

Best regards,
Etsuro Fujita

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To
make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make
changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

file_fdw_notnull_v3.patchapplication/octet-stream; name=file_fdw_notnull_v3.patchDownload

diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 2d36a72..0369aab 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -71,6 +71,9 @@ static const struct FileFdwOption valid_options[] = {
 	 * force_quote is not supported by file_fdw because it's for COPY TO.
 	 */
 
+	/* Validation options */
+	{"validate_data_file", ForeignTableRelationId},
+
 	/* Sentinel */
 	{NULL, InvalidOid}
 };
@@ -94,6 +97,8 @@ typedef struct FileFdwExecutionState
 	char	   *filename;		/* file to read */
 	List	   *options;		/* merged COPY options, excluding filename */
 	CopyState	cstate;			/* state of reading file */
+	bool		validate_data_file;
+								/* whether to validate NOT NULL constraints */
 } FileFdwExecutionState;
 
 /*
@@ -134,7 +139,9 @@ static bool fileAnalyzeForeignTable(Relation relation,
  */
 static bool is_valid_option(const char *option, Oid context);
 static void fileGetOptions(Oid foreigntableid,
-			   char **filename, List **other_options);
+						   char **filename,
+						   List **other_options,
+						   bool *validate_data_file);
 static List *get_file_fdw_attribute_options(Oid relid);
 static void estimate_size(PlannerInfo *root, RelOptInfo *baserel,
 			  FileFdwPlanState *fdw_private);
@@ -182,6 +189,7 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 	char	   *filename = NULL;
 	DefElem	   *force_not_null = NULL;
 	List	   *other_options = NIL;
+	DefElem	   *validate_data_file = NULL;
 	ListCell   *cell;
 
 	/*
@@ -235,9 +243,9 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 		}
 
 		/*
-		 * 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.)
+		 * Separate out filename, force_not_null and validate_data_file, since
+		 * ProcessCopyOptions won't accept them.  (force_not_null only comes in
+		 * a boolean per-column flavor here.)
 		 */
 		if (strcmp(def->defname, "filename") == 0)
 		{
@@ -257,6 +265,16 @@ file_fdw_validator(PG_FUNCTION_ARGS)
 			/* Don't care what the value is, as long as it's a legal boolean */
 			(void) defGetBoolean(def);
 		}
+		else if (strcmp(def->defname, "validate_data_file") == 0)
+		{
+			if (validate_data_file)
+				ereport(ERROR,
+						(errcode(ERRCODE_SYNTAX_ERROR),
+						 errmsg("conflicting or redundant options")));
+			validate_data_file = 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);
 	}
@@ -297,19 +315,23 @@ is_valid_option(const char *option, Oid context)
 /*
  * Fetch the options for a file_fdw foreign table.
  *
- * We have to separate out "filename" from the other options because
- * it must not appear in the options list passed to the core COPY code.
+ * We have to separate out "filename" and "validate_data_file" from the other
+ * options because it must not appear in the options list passed to the core
+ * COPY code.
  */
 static void
 fileGetOptions(Oid foreigntableid,
-			   char **filename, List **other_options)
+			   char **filename,
+			   List **other_options,
+			   bool *validate_data_file)
 {
 	ForeignTable *table;
 	ForeignServer *server;
 	ForeignDataWrapper *wrapper;
 	List	   *options;
 	ListCell   *lc,
-			   *prev;
+			   *prev,
+			   *next;
 
 	/*
 	 * Extract options from FDW objects.  We ignore user mappings because
@@ -330,21 +352,28 @@ fileGetOptions(Oid foreigntableid,
 	options = list_concat(options, get_file_fdw_attribute_options(foreigntableid));
 
 	/*
-	 * Separate out the filename.
+	 * Separate out the filename and data file validation option.
 	 */
 	*filename = NULL;
+	*validate_data_file = false;
 	prev = NULL;
-	foreach(lc, options)
+	for (lc = list_head(options); lc; lc = next)
 	{
 		DefElem    *def = (DefElem *) lfirst(lc);
 
+		next = lnext(lc);
 		if (strcmp(def->defname, "filename") == 0)
 		{
 			*filename = defGetString(def);
 			options = list_delete_cell(options, lc, prev);
-			break;
 		}
-		prev = lc;
+		else if (strcmp(def->defname, "validate_data_file") == 0)
+		{
+			*validate_data_file = defGetBoolean(def);
+			options = list_delete_cell(options, lc, prev);
+		}
+		else
+			prev = lc;
 	}
 
 	/*
@@ -425,6 +454,7 @@ fileGetForeignRelSize(PlannerInfo *root,
 					  Oid foreigntableid)
 {
 	FileFdwPlanState *fdw_private;
+	bool validate_data_file;
 
 	/*
 	 * Fetch options.  We only need filename at this point, but we might
@@ -432,7 +462,9 @@ fileGetForeignRelSize(PlannerInfo *root,
 	 */
 	fdw_private = (FileFdwPlanState *) palloc(sizeof(FileFdwPlanState));
 	fileGetOptions(foreigntableid,
-				   &fdw_private->filename, &fdw_private->options);
+				   &fdw_private->filename,
+				   &fdw_private->options,
+				   &validate_data_file);
 	baserel->fdw_private = (void *) fdw_private;
 
 	/* Estimate relation size */
@@ -517,10 +549,13 @@ fileExplainForeignScan(ForeignScanState *node, ExplainState *es)
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate_data_file;
 
 	/* Fetch options --- we only need filename at this point */
 	fileGetOptions(RelationGetRelid(node->ss.ss_currentRelation),
-				   &filename, &options);
+				   &filename,
+				   &options,
+				   &validate_data_file);
 
 	ExplainPropertyText("Foreign File", filename, es);
 
@@ -544,6 +579,7 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate_data_file;
 	CopyState	cstate;
 	FileFdwExecutionState *festate;
 
@@ -555,7 +591,9 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 
 	/* Fetch options of foreign table */
 	fileGetOptions(RelationGetRelid(node->ss.ss_currentRelation),
-				   &filename, &options);
+				   &filename,
+				   &options,
+				   &validate_data_file);
 
 	/*
 	 * Create CopyState from FDW options.  We always acquire all columns, so
@@ -574,6 +612,7 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
 	festate->filename = filename;
 	festate->options = options;
 	festate->cstate = cstate;
+	festate->validate_data_file = validate_data_file;
 
 	node->fdw_state = (void *) festate;
 }
@@ -588,6 +627,8 @@ fileIterateForeignScan(ForeignScanState *node)
 {
 	FileFdwExecutionState *festate = (FileFdwExecutionState *) node->fdw_state;
 	TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
+	Relation	rel = node->ss.ss_currentRelation;
+	TupleConstr *constr = rel->rd_att->constr;
 	bool		found;
 	ErrorContextCallback errcontext;
 
@@ -614,7 +655,11 @@ fileIterateForeignScan(ForeignScanState *node)
 						 slot->tts_values, slot->tts_isnull,
 						 NULL);
 	if (found)
+	{
 		ExecStoreVirtualTuple(slot);
+		if (constr && constr->has_not_null && festate->validate_data_file)
+			ExecNotNullCheck(rel, slot);
+	}
 
 	/* Remove error callback. */
 	error_context_stack = errcontext.previous;
@@ -664,10 +709,14 @@ fileAnalyzeForeignTable(Relation relation,
 {
 	char	   *filename;
 	List	   *options;
+	bool		validate_data_file;
 	struct stat	stat_buf;
 
 	/* Fetch options of foreign table */
-	fileGetOptions(RelationGetRelid(relation), &filename, &options);
+	fileGetOptions(RelationGetRelid(relation),
+				   &filename,
+				   &options,
+				   &validate_data_file);
 
 	/*
 	 * Get size of the file.  (XXX if we fail here, would it be better to
@@ -831,6 +880,7 @@ file_acquire_sample_rows(Relation onerel, int elevel,
 	bool		found;
 	char	   *filename;
 	List	   *options;
+	bool		validate_data_file;
 	CopyState	cstate;
 	ErrorContextCallback errcontext;
 	MemoryContext oldcontext = CurrentMemoryContext;
@@ -844,7 +894,10 @@ file_acquire_sample_rows(Relation onerel, int elevel,
 	nulls = (bool *) palloc(tupDesc->natts * sizeof(bool));
 
 	/* Fetch options of foreign table */
-	fileGetOptions(RelationGetRelid(onerel), &filename, &options);
+	fileGetOptions(RelationGetRelid(onerel),
+				   &filename,
+				   &options,
+				   &validate_data_file);
 
 	/*
 	 * Create CopyState from FDW options.
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
index 84f0750..1232e5d 100644
--- a/contrib/file_fdw/output/file_fdw.source
+++ b/contrib/file_fdw/output/file_fdw.source
@@ -123,7 +123,7 @@ ERROR:  invalid option "force_not_null"
 HINT:  Valid options in this context are: 
 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
+HINT:  Valid options in this context are: filename, format, header, delimiter, quote, escape, null, encoding, validate_data_file
 -- 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
index 4c7f7e8..4816042 100644
--- a/doc/src/sgml/file-fdw.sgml
+++ b/doc/src/sgml/file-fdw.sgml
@@ -108,6 +108,16 @@
    </listitem>
   </varlistentry>
 
+  <varlistentry>
+   <term><literal>validate_data_file</literal></term>
+
+   <listitem>
+    <para>
+     Specifies whether the <literal>NOT NULL</literal> constraint is validated.
+    </para>
+   </listitem>
+  </varlistentry>
+
  </variablelist>
 
  <para>
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index fbb36fa..2de72df 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1438,6 +1438,28 @@ ExecutePlan(EState *estate,
 
 
 /*
+ * ExecNotNullCheck --- check that tuple meets NOT NULL constraints
+ */
+void
+ExecNotNullCheck(Relation rel, TupleTableSlot *slot)
+{
+	int			natts = rel->rd_att->natts;
+	int			attrChk;
+
+	for (attrChk = 1; attrChk <= natts; attrChk++)
+	{
+		if (rel->rd_att->attrs[attrChk - 1]->attnotnull &&
+			slot_attisnull(slot, attrChk))
+			ereport(ERROR,
+					(errcode(ERRCODE_NOT_NULL_VIOLATION),
+					 errmsg("null value in column \"%s\" violates not-null constraint",
+							NameStr(rel->rd_att->attrs[attrChk - 1]->attname)),
+					 errdetail("Failing row contains %s.",
+							   ExecBuildSlotValueDescription(slot, 64))));
+	}
+}
+
+/*
  * ExecRelCheck --- check that tuple meets constraints for result relation
  */
 static const char *
@@ -1509,22 +1531,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 	Assert(constr);
 
 	if (constr->has_not_null)
-	{
-		int			natts = rel->rd_att->natts;
-		int			attrChk;
-
-		for (attrChk = 1; attrChk <= natts; attrChk++)
-		{
-			if (rel->rd_att->attrs[attrChk - 1]->attnotnull &&
-				slot_attisnull(slot, attrChk))
-				ereport(ERROR,
-						(errcode(ERRCODE_NOT_NULL_VIOLATION),
-						 errmsg("null value in column \"%s\" violates not-null constraint",
-						NameStr(rel->rd_att->attrs[attrChk - 1]->attname)),
-						 errdetail("Failing row contains %s.",
-								   ExecBuildSlotValueDescription(slot, 64))));
-		}
-	}
+		ExecNotNullCheck(rel, slot);
 
 	if (constr->num_check > 0)
 	{
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f5503a5..5eda8ea 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -188,6 +188,7 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
 extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
+extern void ExecNotNullCheck(Relation rel, TupleTableSlot *slot);
 extern void ExecConstraints(ResultRelInfo *resultRelInfo,
 				TupleTableSlot *slot, EState *estate);
 extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti);

Etsuro Fujita

fujita.etsuro@lab.ntt.co.jp

over 13 years ago

In reply to: Etsuro Fujita (#5)

Re: not null validation option in contrib/file_fdw

I rebased the patch to current head. Attached is an updated version of the
patch.

Best regards,
Etsuro Fujita

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Etsuro Fujita
Sent: Tuesday, April 17, 2012 2:40 PM
To: 'Andrew Dunstan'; 'Shigeru HANADA'
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in contrib/file_fdw

I updated the patch. Attached is an updated version of the patch.

Changes:
* fix a bug in fileGetOptions()
* rename the validation option and its code to "validate_data_file"
* clean up

Best regards,
Etsuro Fujita

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Etsuro Fujita
Sent: Monday, April 16, 2012 4:09 PM
To: 'Andrew Dunstan'; 'Shigeru HANADA'
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in contrib/file_fdw

Thank you for the review.

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Andrew
Dunstan
Sent: Friday, April 13, 2012 9:16 PM
To: Shigeru HANADA
Cc: Etsuro Fujita; pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] not null validation option in
contrib/file_fdw

On 04/13/2012 07:21 AM, Shigeru HANADA wrote:

(2012/04/13 16:59), Etsuro Fujita wrote:

I updated the patch added to CF 2012-Next [1]. Attached is the
updated version of the patch.

I applied the patch and ran regression tests of file_fdw, and I
got SIGSEGV X-(

The failure occurs in fileGetOptions, and it is caused by
list_delete_cell used in foreach loop; ListCell points delete
target has been free-ed in list_delete_cell, but foreach accesses
it to get next element.

Some of backend functions which use list_delete_cell in loop use

"for"

loop instead of foreach, and other functions exit the loop after
calling list_delete_cell. Since we can't stop searching non-COPY
options until meeting the end of the options list, we would need
to choose former ("for" loop), or create another list which
contains only valid COPY options and return it via other_options

parameter.

Yes, the code in fileGetOptions() appears to be bogus.

Sorry, I will fix it.

Also, "validate" is a terrible name for the option (and in the code)

IMNSHO.

It's far too generic. "validate_not_null" or some such would surely
be better.

I thought it would be used for not only NOT NULL but also CHECK and

foreign

key constraints. That is, when a user sets the option to 'true',
file_fdw verifies that each tuple meets all kinds of constraints. So,
how about "validate_data_file" or simply "validate_file"?

Best regards,
Etsuro Fujita

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To
make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

begin 666 file_fdw_notnull_v4.patch
M9&EF9B M+6=I="!A+V-O;G1R:6(O9FEL95]F9'<O9FEL95]F9'<N8R!B+V-O
M;G1R:6(O9FEL95]F9'<O9FEL95]F9'<N8PII;F1E>"!E,V(Y,C(S+BXQ.3$X
M-6(X(#$P,#8T- HM+2T@82]C;VYT<FEB+V9I;&5?9F1W+V9I;&5?9F1W+F,*
M*RLK(&(O8V]N=')I8B]F:6QE7V9D=R]F:6QE7V9D=RYC"D! ("TW,2PV("LW
M,2PY($! ('-T871I8R!C;VYS="!S=')U8W0@1FEL949D=T]P=&EO;B!V86QI
M9%]O<'1I;VYS6UT@/2!["B )("H@9F]R8V5?<75O=&4@:7,@;F]T('-U<'!O
M<G1E9"!B>2!F:6QE7V9D=R!B96-A=7-E(&ET)W,@9F]R($-/4%D@5$\N"B )
M("HO"B **PDO*B!686QI9&%T:6]N(&]P=&EO;G,@*B\**PE[(G9A;&ED871E
M7V1A=&%?9FEL92(L($9O<F5I9VY486)L95)E;&%T:6]N261]+ HK"B )+RH@
M4V5N=&EN96P@*B\*( E[3E5,3"P@26YV86QI9$]I9'T*('T["D! ("TY-"PV
M("LY-RPX($! ('1Y<&5D968@<W1R=6-T($9I;&5&9'=%>&5C=71I;VY3=&%T
M90H@"6-H87()(" @*F9I;&5N86UE.PD)+RH@9FEL92!T;R!R96%D("HO"B )
M3&ES= D@(" J;W!T:6]N<SL)"2\J(&UE<F=E9"!#3U!9(&]P=&EO;G,L(&5X
M8VQU9&EN9R!F:6QE;F%M92 J+PH@"4-O<'E3=&%T90EC<W1A=&4["0D)+RH@
M<W1A=&4@;V8@<F5A9&EN9R!F:6QE("HO"BL)8F]O; D)=F%L:61A=&5?9&%T
M85]F:6QE.PHK"0D)"0D)"0DO*B!W:&5T:&5R('1O('9A;&ED871E($Y/5"!.
M54Q,(&-O;G-T<F%I;G1S("HO"B!]($9I;&5&9'=%>&5C=71I;VY3=&%T93L*
M( H@+RH*0$ @+3$S-"PW("LQ,SDL.2! 0"!S=&%T:6,@8F]O;"!F:6QE06YA
M;'EZ949O<F5I9VY486)L92A296QA=&EO;B!R96QA=&EO;BP*(" J+PH@<W1A
M=&EC(&)O;VP@:7-?=F%L:61?;W!T:6]N*&-O;G-T(&-H87(@*F]P=&EO;BP@
M3VED(&-O;G1E>'0I.PH@<W1A=&EC('9O:60@9FEL94=E=$]P=&EO;G,H3VED
M(&9O<F5I9VYT86)L96ED+ HM"0D)(" @8VAA<B J*F9I;&5N86UE+"!,:7-T
M("HJ;W1H97)?;W!T:6]N<RD["BL)"0D)"0D@("!C:&%R("HJ9FEL96YA;64L
M"BL)"0D)"0D@("!,:7-T("HJ;W1H97)?;W!T:6]N<RP**PD)"0D)"2 @(&)O
M;VP@*G9A;&ED871E7V1A=&%?9FEL92D["B!S=&%T:6,@3&ES=" J9V5T7V9I
M;&5?9F1W7V%T=')I8G5T95]O<'1I;VYS*$]I9"!R96QI9"D["B!S=&%T:6,@
M=F]I9"!E<W1I;6%T95]S:7IE*%!L86YN97));F9O("IR;V]T+"!296Q/<'1)
M;F9O("IB87-E<F5L+ H@"0D)("!&:6QE1F1W4&QA;E-T871E("IF9'=?<')I
M=F%T92D["D! ("TQ.#(L-B K,3@Y+#<@0$ @9FEL95]F9'=?=F%L:61A=&]R
M*%!'7T953D-424].7T%21U,I"B )8VAA<@D@(" J9FEL96YA;64@/2!.54Q,
M.PH@"41E9D5L96T@(" @*F9O<F-E7VYO=%]N=6QL(#T@3E5,3#L*( E,:7-T
M"2 @("IO=&AE<E]O<'1I;VYS(#T@3DE,.PHK"41E9D5L96T)(" @*G9A;&ED
M871E7V1A=&%?9FEL92 ]($Y53$P["B )3&ES=$-E;&P@(" J8V5L;#L*( H@
M"2\J"D! ("TR,S<L.2 K,C0U+#D@0$ @9FEL95]F9'=?=F%L:61A=&]R*%!'
M7T953D-424].7T%21U,I"B )"7T*( H@"0DO*@HM"0D@*B!397!A<F%T92!O
M=70@9FEL96YA;64@86YD(&9O<F-E7VYO=%]N=6QL+"!S:6YC92!0<F]C97-S
M0V]P>4]P=&EO;G,*+0D)("H@=V]N)W0@86-C97!T('1H96TN(" H9F]R8V5?
M;F]T7VYU;&P@;VYL>2!C;VUE<R!I;B!A(&)O;VQE86X*+0D)("H@<&5R+6-O
M;'5M;B!F;&%V;W(@:&5R92XI"BL)"2 J(%-E<&%R871E(&]U="!F:6QE;F%M
M92P@9F]R8V5?;F]T7VYU;&P@86YD('9A;&ED871E7V1A=&%?9FEL92P@<VEN
M8V4**PD)("H@4')O8V5S<T-O<'E/<'1I;VYS('=O;B=T(&%C8V5P="!T:&5M
M+B @*&9O<F-E7VYO=%]N=6QL(&]N;'D@8V]M97,@:6X**PD)("H@82!B;V]L
M96%N('!E<BUC;VQU;6X@9FQA=F]R(&AE<F4N*0H@"0D@*B\*( D):68@*'-T
M<F-M<"AD968M/F1E9FYA;64L(")F:6QE;F%M92(I(#T](# I"B )"7L*0$ @
M+3(U.2PV("LR-C<L,38@0$ @9FEL95]F9'=?=F%L:61A=&]R*%!'7T953D-4
M24].7T%21U,I"B )"0DO*B!$;VXG="!C87)E('=H870@=&AE('9A;'5E(&ES
M+"!A<R!L;VYG(&%S(&ET)W,@82!L96=A;"!B;V]L96%N("HO"B )"0DH=F]I
M9"D@9&5F1V5T0F]O;&5A;BAD968I.PH@"0E]"BL)"65L<V4@:68@*'-T<F-M
M<"AD968M/F1E9FYA;64L(")V86QI9&%T95]D871A7V9I;&4B*2 ]/2 P*0HK
M"0E["BL)"0EI9B H=F%L:61A=&5?9&%T85]F:6QE*0HK"0D)"65R97!O<G0H
M15)23U(L"BL)"0D)"0DH97)R8V]D92A%4E)#3T1%7U-93E1!6%]%4E)/4BDL
M"BL)"0D)"0D@97)R;7-G*")C;VYF;&EC=&EN9R!O<B!R961U;F1A;G0@;W!T
M:6]N<R(I*2D["BL)"0EV86QI9&%T95]D871A7V9I;&4@/2!D968["BL)"0DO
M*B!$;VXG="!C87)E('=H870@=&AE('9A;'5E(&ES+"!A<R!L;VYG(&%S(&ET
M)W,@82!L96=A;"!B;V]L96%N("HO"BL)"0DH=F]I9"D@9&5F1V5T0F]O;&5A
M;BAD968I.PHK"0E]"B )"65L<V4*( D)"6]T:&5R7V]P=&EO;G,@/2!L87!P
M96YD*&]T:&5R7V]P=&EO;G,L(&1E9BD["B )?0I 0" M,CDY+#$Y("LS,3<L
M,C,@0$ @:7-?=F%L:61?;W!T:6]N*&-O;G-T(&-H87(@*F]P=&EO;BP@3VED
M(&-O;G1E>'0I"B O*@H@("H@1F5T8V@@=&AE(&]P=&EO;G,@9F]R(&$@9FEL
M95]F9'<@9F]R96EG;B!T86)L92X*(" J"BT@*B!792!H879E('1O('-E<&%R
M871E(&]U=" B9FEL96YA;64B(&9R;VT@=&AE(&]T:&5R(&]P=&EO;G,@8F5C
M875S90HM("H@:70@;75S="!N;W0@87!P96%R(&EN('1H92!O<'1I;VYS(&QI
M<W0@<&%S<V5D('1O('1H92!C;W)E($-/4%D@8V]D92X**R J(%=E(&AA=F4@
M=&\@<V5P87)A=&4@;W5T(")F:6QE;F%M92(@86YD(")V86QI9&%T95]D871A
M7V9I;&4B(&9R;VT@=&AE(&]T:&5R"BL@*B!O<'1I;VYS(&)E8V%U<V4@:70@
M;75S="!N;W0@87!P96%R(&EN('1H92!O<'1I;VYS(&QI<W0@<&%S<V5D('1O
M('1H92!C;W)E"BL@*B!#3U!9(&-O9&4N"B @*B\*('-T871I8R!V;VED"B!F
M:6QE1V5T3W!T:6]N<RA/:60@9F]R96EG;G1A8FQE:60L"BT)"0D@("!C:&%R
M("HJ9FEL96YA;64L($QI<W0@*BIO=&AE<E]O<'1I;VYS*0HK"0D)(" @8VAA
M<B J*F9I;&5N86UE+ HK"0D)(" @3&ES=" J*F]T:&5R7V]P=&EO;G,L"BL)
M"0D@("!B;V]L("IV86QI9&%T95]D871A7V9I;&4I"B!["B )1F]R96EG;E1A
M8FQE("IT86)L93L*( E&;W)E:6=N4V5R=F5R("IS97)V97(["B )1F]R96EG
M;D1A=&%7<F%P<&5R("IW<F%P<&5R.PH@"4QI<W0)(" @*F]P=&EO;G,["B )
M3&ES=$-E;&P@(" J;&,L"BT)"0D@(" J<')E=CL**PD)"2 @("IP<F5V+ HK
M"0D)(" @*FYE>'0["B *( DO*@H@"2 J($5X=')A8W0@;W!T:6]N<R!F<F]M
M($9$5R!O8FIE8W1S+B @5V4@:6=N;W)E('5S97(@;6%P<&EN9W,@8F5C875S
M90I 0" M,S,R+#(Q("LS-30L,S @0$ @9FEL94=E=$]P=&EO;G,H3VED(&9O
M<F5I9VYT86)L96ED+ H@"6]P=&EO;G,@/2!L:7-T7V-O;F-A="AO<'1I;VYS
M+"!G971?9FEL95]F9'=?871T<FEB=71E7V]P=&EO;G,H9F]R96EG;G1A8FQE
M:60I*3L*( H@"2\J"BT)("H@4V5P87)A=&4@;W5T('1H92!F:6QE;F%M92X*
M*PD@*B!397!A<F%T92!O=70@=&AE(&9I;&5N86UE(&%N9"!D871A(&9I;&4@
M=F%L:61A=&EO;B!O<'1I;VXN"B )("HO"B )*F9I;&5N86UE(#T@3E5,3#L*
M*PEI9B H=F%L:61A=&5?9&%T85]F:6QE*0HK"0DJ=F%L:61A=&5?9&%T85]F
M:6QE(#T@9F%L<V4["B )<')E=B ]($Y53$P["BT)9F]R96%C:"AL8RP@;W!T
M:6]N<RD**PEF;W(@*&QC(#T@;&ES=%]H96%D*&]P=&EO;G,I.R!L8SL@;&,@
M/2!N97AT*0H@"7L*( D)1&5F16QE;2 @(" J9&5F(#T@*$1E9D5L96T@*BD@
M;&9I<G-T*&QC*3L*( HK"0EN97AT(#T@;&YE>'0H;&,I.PH@"0EI9B H<W1R
M8VUP*&1E9BT^9&5F;F%M92P@(F9I;&5N86UE(BD@/3T@,"D*( D)>PH@"0D)
M*F9I;&5N86UE(#T@9&5F1V5T4W1R:6YG*&1E9BD["B )"0EO<'1I;VYS(#T@
M;&ES=%]D96QE=&5?8V5L;"AO<'1I;VYS+"!L8RP@<')E=BD["BT)"0EB<F5A
M:SL*( D)?0HM"0EP<F5V(#T@;&,["BL)"65L<V4@:68@*'-T<F-M<"AD968M
M/F1E9FYA;64L(")V86QI9&%T95]D871A7V9I;&4B*2 ]/2 P*0HK"0E["BL)
M"0EI9B H=F%L:61A=&5?9&%T85]F:6QE*0HK"0D)"2IV86QI9&%T95]D871A
M7V9I;&4@/2!D969'971";V]L96%N*&1E9BD["BL)"0EO<'1I;VYS(#T@;&ES
M=%]D96QE=&5?8V5L;"AO<'1I;VYS+"!L8RP@<')E=BD["BL)"7T**PD)96QS
M90HK"0D)<')E=B ](&QC.PH@"7T*( H@"2\J"D! ("TT,S0L-R K-#8U+#D@
M0$ @9FEL94=E=$9O<F5I9VY296Q3:7IE*%!L86YN97));F9O("IR;V]T+ H@
M"2 J+PH@"69D=U]P<FEV871E(#T@*$9I;&5&9'=0;&%N4W1A=&4@*BD@<&%L
M;&]C*'-I>F5O9BA&:6QE1F1W4&QA;E-T871E*2D["B )9FEL94=E=$]P=&EO
M;G,H9F]R96EG;G1A8FQE:60L"BT)"0D)(" @)F9D=U]P<FEV871E+3YF:6QE
M;F%M92P@)F9D=U]P<FEV871E+3YO<'1I;VYS*3L**PD)"0D@(" F9F1W7W!R
M:79A=&4M/F9I;&5N86UE+ HK"0D)"2 @("9F9'=?<')I=F%T92T^;W!T:6]N
M<RP**PD)"0D@("!.54Q,*3L*( EB87-E<F5L+3YF9'=?<')I=F%T92 ]("AV
M;VED("HI(&9D=U]P<FEV871E.PH@"B )+RH@17-T:6UA=&4@<F5L871I;VX@
M<VEZ92 J+PI 0" M-3(R+#<@*S4U-2PY($! (&9I;&5%>'!L86EN1F]R96EG
M;E-C86XH1F]R96EG;E-C86Y3=&%T92 J;F]D92P@17AP;&%I;E-T871E("IE
M<RD*( H@"2\J($9E=&-H(&]P=&EO;G,@+2TM('=E(&]N;'D@;F5E9"!F:6QE
M;F%M92!A="!T:&ES('!O:6YT("HO"B )9FEL94=E=$]P=&EO;G,H4F5L871I
M;VY'971296QI9"AN;V1E+3YS<RYS<U]C=7)R96YT4F5L871I;VXI+ HM"0D)
M"2 @("9F:6QE;F%M92P@)F]P=&EO;G,I.PHK"0D)"2 @("9F:6QE;F%M92P*
M*PD)"0D@(" F;W!T:6]N<RP**PD)"0D@("!.54Q,*3L*( H@"45X<&QA:6Y0
M<F]P97)T>51E>'0H(D9O<F5I9VX@1FEL92(L(&9I;&5N86UE+"!E<RD["B *
M0$ @+34T-RPV("LU.#(L-R! 0"!F:6QE0F5G:6Y&;W)E:6=N4V-A;BA&;W)E
M:6=N4V-A;E-T871E("IN;V1E+"!I;G0@969L86=S*0H@"6-H87()(" @*F9I
M;&5N86UE.PH@"4QI<W0)(" @*F]P=&EO;G,["B )0V]P>5-T871E"6-S=&%T
M93L**PEB;V]L"0EV86QI9&%T95]D871A7V9I;&4["B )1FEL949D=T5X96-U
M=&EO;E-T871E("IF97-T871E.PH@"B )+RH*0$ @+34U-RPW("LU.3,L.2!
M0"!F:6QE0F5G:6Y&;W)E:6=N4V-A;BA&;W)E:6=N4V-A;E-T871E("IN;V1E
M+"!I;G0@969L86=S*0H@"B )+RH@1F5T8V@@;W!T:6]N<R!O9B!F;W)E:6=N
M('1A8FQE("HO"B )9FEL94=E=$]P=&EO;G,H4F5L871I;VY'971296QI9"AN
M;V1E+3YS<RYS<U]C=7)R96YT4F5L871I;VXI+ HM"0D)"2 @("9F:6QE;F%M
M92P@)F]P=&EO;G,I.PHK"0D)"2 @("9F:6QE;F%M92P**PD)"0D@(" F;W!T
M:6]N<RP**PD)"0D@(" F=F%L:61A=&5?9&%T85]F:6QE*3L*( H@"2\J"B )
M("H@0W)E871E($-O<'E3=&%T92!F<F]M($9$5R!O<'1I;VYS+B @5V4@86QW
M87ES(&%C<75I<F4@86QL(&-O;'5M;G,L('-O"D! ("TU-S8L-B K-C$T+#<@
M0$ @9FEL94)E9VEN1F]R96EG;E-C86XH1F]R96EG;E-C86Y3=&%T92 J;F]D
M92P@:6YT(&5F;&%G<RD*( EF97-T871E+3YF:6QE;F%M92 ](&9I;&5N86UE
M.PH@"69E<W1A=&4M/F]P=&EO;G,@/2!O<'1I;VYS.PH@"69E<W1A=&4M/F-S
M=&%T92 ](&-S=&%T93L**PEF97-T871E+3YV86QI9&%T95]D871A7V9I;&4@
M/2!V86QI9&%T95]D871A7V9I;&4["B *( EN;V1E+3YF9'=?<W1A=&4@/2 H
M=F]I9" J*2!F97-T871E.PH@?0I 0" M-3DP+#8@*S8R.2PX($! (&9I;&5)
M=&5R871E1F]R96EG;E-C86XH1F]R96EG;E-C86Y3=&%T92 J;F]D92D*('L*
M( E&:6QE1F1W17AE8W5T:6]N4W1A=&4@*F9E<W1A=&4@/2 H1FEL949D=T5X
M96-U=&EO;E-T871E("HI(&YO9&4M/F9D=U]S=&%T93L*( E4=7!L951A8FQE
M4VQO=" J<VQO=" ](&YO9&4M/G-S+G-S7U-C86Y4=7!L95-L;W0["BL)4F5L
M871I;VX)<F5L(#T@;F]D92T^<W,N<W-?8W5R<F5N=%)E;&%T:6]N.PHK"51U
M<&QE0V]N<W1R("IC;VYS='(@/2!R96PM/G)D7V%T="T^8V]N<W1R.PH@"6)O
M;VP)"69O=6YD.PH@"45R<F]R0V]N=&5X=$-A;&QB86-K(&5R<F-O;G1E>'0[
M"B *0$ @+38Q-BPW("LV-3<L,3$@0$ @9FEL94ET97)A=&5&;W)E:6=N4V-A
M;BA&;W)E:6=N4V-A;E-T871E("IN;V1E*0H@"0D)"0D)('-L;W0M/G1T<U]V
M86QU97,L('-L;W0M/G1T<U]I<VYU;&PL"B )"0D)"0D@3E5,3"D["B ):68@
M*&9O=6YD*0HK"7L*( D)17AE8U-T;W)E5FER='5A;%1U<&QE*'-L;W0I.PHK
M"0EI9B H8V]N<W1R("8F(&-O;G-T<BT^:&%S7VYO=%]N=6QL("8F(&9E<W1A
M=&4M/G9A;&ED871E7V1A=&%?9FEL92D**PD)"45X96-.;W1.=6QL0VAE8VLH
M<F5L+"!S;&]T*3L**PE]"B *( DO*B!296UO=F4@97)R;W(@8V%L;&)A8VLN
M("HO"B )97)R;W)?8V]N=&5X=%]S=&%C:R ](&5R<F-O;G1E>'0N<')E=FEO
M=7,["D! ("TV-CDL-R K-S$T+#<@0$ @9FEL94%N86QY>F5&;W)E:6=N5&%B
M;&4H4F5L871I;VX@<F5L871I;VXL"B )<W1R=6-T('-T870@<W1A=%]B=68[
M"B *( DO*B!&971C:"!O<'1I;VYS(&]F(&9O<F5I9VX@=&%B;&4@*B\*+0EF
M:6QE1V5T3W!T:6]N<RA296QA=&EO;D=E=%)E;&ED*')E;&%T:6]N*2P@)F9I
M;&5N86UE+" F;W!T:6]N<RD["BL)9FEL94=E=$]P=&EO;G,H4F5L871I;VY'
M971296QI9"AR96QA=&EO;BDL("9F:6QE;F%M92P@)F]P=&EO;G,L($Y53$PI
M.PH@"B )+RH*( D@*B!'970@<VEZ92!O9B!T:&4@9FEL92X@("A86%@@:68@
M=V4@9F%I;"!H97)E+"!W;W5L9"!I="!B92!B971T97(@=&\@:G5S= I 0" M
M.#0V+#<@*S@Y,2PW($! (&9I;&5?86-Q=6ER95]S86UP;&5?<F]W<RA296QA
M=&EO;B!O;F5R96PL(&EN="!E;&5V96PL"B );G5L;',@/2 H8F]O;" J*2!P
M86QL;V,H='5P1&5S8RT^;F%T=',@*B!S:7IE;V8H8F]O;"DI.PH@"B )+RH@
M1F5T8V@@;W!T:6]N<R!O9B!F;W)E:6=N('1A8FQE("HO"BT)9FEL94=E=$]P
M=&EO;G,H4F5L871I;VY'971296QI9"AO;F5R96PI+" F9FEL96YA;64L("9O
M<'1I;VYS*3L**PEF:6QE1V5T3W!T:6]N<RA296QA=&EO;D=E=%)E;&ED*&]N
M97)E;"DL("9F:6QE;F%M92P@)F]P=&EO;G,L($Y53$PI.PH@"B )+RH*( D@
M*B!#<F5A=&4@0V]P>5-T871E(&9R;VT@1D17(&]P=&EO;G,N"F1I9F8@+2UG
M:70@82]C;VYT<FEB+V9I;&5?9F1W+V]U='!U="]F:6QE7V9D=RYS;W5R8V4@
M8B]C;VYT<FEB+V9I;&5?9F1W+V]U='!U="]F:6QE7V9D=RYS;W5R8V4*:6YD
M97@@-F8Y,#9E,2XN,C5A93EF-R Q,# V-#0*+2TM(&$O8V]N=')I8B]F:6QE
M7V9D=R]O=71P=70O9FEL95]F9'<N<V]U<F-E"BLK*R!B+V-O;G1R:6(O9FEL
M95]F9'<O;W5T<'5T+V9I;&5?9F1W+G-O=7)C90I 0" M,3(S+#<@*S$R,RPW
M($! ($524D]2.B @:6YV86QI9"!O<'1I;VX@(F9O<F-E7VYO=%]N=6QL(@H@
M2$E.5#H@(%1H97)E(&%R92!N;R!V86QI9"!O<'1I;VYS(&EN('1H:7,@8V]N
M=&5X="X*($-214%412!&3U)%24=.(%1!0DQ%('1B;" H*2!315)615(@9FEL
M95]S97)V97(@3U!424].4R H9F]R8V5?;F]T7VYU;&P@)RHG*3L@+2T@15)2
M3U(*($524D]2.B @:6YV86QI9"!O<'1I;VX@(F9O<F-E7VYO=%]N=6QL(@HM
M2$E.5#H@(%9A;&ED(&]P=&EO;G,@:6X@=&AI<R!C;VYT97AT(&%R93H@9FEL
M96YA;64L(&9O<FUA="P@:&5A9&5R+"!D96QI;6ET97(L('%U;W1E+"!E<V-A
M<&4L(&YU;&PL(&5N8V]D:6YG"BM(24Y4.B @5F%L:60@;W!T:6]N<R!I;B!T
M:&ES(&-O;G1E>'0@87)E.B!F:6QE;F%M92P@9F]R;6%T+"!H96%D97(L(&1E
M;&EM:71E<BP@<75O=&4L(&5S8V%P92P@;G5L;"P@96YC;V1I;F<L('9A;&ED
M871E7V1A=&%?9FEL90H@+2T@8F%S:6,@<75E<GD@=&5S=',*(%-%3$5#5" J
M($923TT@86=G7W1E>'0@5TA%4D4@8B ^(#$P+C @3U)$15(@0ED@83L*(" @
M82 @?" @(&(@(" @"F1I9F8@+2UG:70@82]D;V,O<W)C+W-G;6PO9FEL92UF
M9'<N<V=M;"!B+V1O8R]S<F,O<V=M;"]F:6QE+69D=RYS9VUL"FEN9&5X(#@X
M.6%A-S<N+C4T-3$X,&4@,3 P-C0T"BTM+2!A+V1O8R]S<F,O<V=M;"]F:6QE
M+69D=RYS9VUL"BLK*R!B+V1O8R]S<F,O<V=M;"]F:6QE+69D=RYS9VUL"D!
M("TQ,#@L-B K,3 X+#$V($! "B @(" \+VQI<W1I=&5M/@H@(" \+W9A<FQI
M<W1E;G1R>3X*( HK(" \=F%R;&ES=&5N=')Y/@HK(" @/'1E<FT^/&QI=&5R
M86P^=F%L:61A=&5?9&%T85]F:6QE/"]L:71E<F%L/CPO=&5R;3X**PHK(" @
M/&QI<W1I=&5M/@HK(" @(#QP87)A/@HK(" @("!3<&5C:69I97,@=VAE=&AE
M<B!T:&4@/&QI=&5R86P^3D]4($Y53$P\+VQI=&5R86P^(&-O;G-T<F%I;G0@
M:7,@=F%L:61A=&5D+@HK(" @(#PO<&%R83X**R @(#PO;&ES=&ET96T^"BL@
M(#PO=F%R;&ES=&5N=')Y/@HK"B @/"]V87)I86)L96QI<W0^"B *(" \<&%R
M83X*9&EF9B M+6=I="!A+W-R8R]B86-K96YD+V5X96-U=&]R+V5X96--86EN
M+F,@8B]S<F,O8F%C:V5N9"]E>&5C=71O<B]E>&5C36%I;BYC"FEN9&5X(#0T
M,#0S.&(N+F8Q,#=B,S$@,3 P-C0T"BTM+2!A+W-R8R]B86-K96YD+V5X96-U
M=&]R+V5X96--86EN+F,**RLK(&(O<W)C+V)A8VME;F0O97AE8W5T;W(O97AE
M8TUA:6XN8PI 0" M,30S."PV("LQ-#,X+#(X($! ($5X96-U=&50;&%N*$53
M=&%T92 J97-T871E+ H@"B *("\J"BL@*B!%>&5C3F]T3G5L;$-H96-K("TM
M+2!C:&5C:R!T:&%T('1U<&QE(&UE971S($Y/5"!.54Q,(&-O;G-T<F%I;G1S
M"BL@*B\**W9O:60**T5X96-.;W1.=6QL0VAE8VLH4F5L871I;VX@<F5L+"!4
M=7!L951A8FQE4VQO=" J<VQO="D**WL**PEI;G0)"0EN871T<R ](')E;"T^
M<F1?871T+3YN871T<SL**PEI;G0)"0EA='1R0VAK.PHK"BL)9F]R("AA='1R
M0VAK(#T@,3L@871T<D-H:R \/2!N871T<SL@871T<D-H:RLK*0HK"7L**PD)
M:68@*')E;"T^<F1?871T+3YA='1R<UMA='1R0VAK("T@,5TM/F%T=&YO=&YU
M;&P@)B8**PD)"7-L;W1?871T:7-N=6QL*'-L;W0L(&%T=')#:&LI*0HK"0D)
M97)E<&]R="A%4E)/4BP**PD)"0D)*&5R<F-O9&4H15)20T]$15].3U1?3E5,
M3%]624],051)3TXI+ HK"0D)"0D@97)R;7-G*")N=6QL('9A;'5E(&EN(&-O
M;'5M;B!<(B5S7"(@=FEO;&%T97,@;F]T+6YU;&P@8V]N<W1R86EN="(L"BL)
M"0D)"0D)3F%M95-T<BAR96PM/G)D7V%T="T^871T<G-;871T<D-H:R M(#%=
M+3YA='1N86UE*2DL"BL)"0D)"2!E<G)D971A:6PH(D9A:6QI;F<@<F]W(&-O
M;G1A:6YS("5S+B(L"BL)"0D)"0D)(" @17AE8T)U:6QD4VQO=%9A;'5E1&5S
M8W)I<'1I;VXH<VQO="P@-C0I*2DI.PHK"7T**WT**PHK+RH*(" J($5X96-2
M96Q#:&5C:R M+2T@8VAE8VL@=&AA="!T=7!L92!M965T<R!C;VYS=')A:6YT
M<R!F;W(@<F5S=6QT(')E;&%T:6]N"B @*B\*('-T871I8R!C;VYS="!C:&%R
M("H*0$ @+3$U,#DL,C(@*S$U,S$L-R! 0"!%>&5C0V]N<W1R86EN=',H4F5S
M=6QT4F5L26YF;R J<F5S=6QT4F5L26YF;RP*( E!<W-E<G0H8V]N<W1R*3L*
M( H@"6EF("AC;VYS='(M/FAA<U]N;W1?;G5L;"D*+0E["BT)"6EN= D)"6YA
M='1S(#T@<F5L+3YR9%]A='0M/FYA='1S.PHM"0EI;G0)"0EA='1R0VAK.PHM
M"BT)"69O<B H871T<D-H:R ](#$[(&%T=')#:&L@/#T@;F%T=',[(&%T=')#
M:&LK*RD*+0D)>PHM"0D):68@*')E;"T^<F1?871T+3YA='1R<UMA='1R0VAK
M("T@,5TM/F%T=&YO=&YU;&P@)B8*+0D)"0ES;&]T7V%T=&ES;G5L;"AS;&]T
M+"!A='1R0VAK*2D*+0D)"0EE<F5P;W)T*$524D]2+ HM"0D)"0D)*&5R<F-O
M9&4H15)20T]$15].3U1?3E5,3%]624],051)3TXI+ HM"0D)"0D)(&5R<FUS
M9R@B;G5L;"!V86QU92!I;B!C;VQU;6X@7"(E<UPB('9I;VQA=&5S(&YO="UN
M=6QL(&-O;G-T<F%I;G0B+ HM"0D)"0D)("!.86UE4W1R*')E;"T^<F1?871T
M+3YA='1R<UMA='1R0VAK("T@,5TM/F%T=&YA;64I*2P*+0D)"0D)"2!E<G)D
M971A:6PH(D9A:6QI;F<@<F]W(&-O;G1A:6YS("5S+B(L"BT)"0D)"0D)"2 @
M($5X96-"=6EL9%-L;W1686QU941E<V-R:7!T:6]N*'-L;W0L(#8T*2DI*3L*
M+0D)?0HM"7T**PD)17AE8TYO=$YU;&Q#:&5C:RAR96PL('-L;W0I.PH@"B )
M:68@*&-O;G-T<BT^;G5M7V-H96-K(#X@,"D*( E["F1I9F8@+2UG:70@82]S
M<F,O:6YC;'5D92]E>&5C=71O<B]E>&5C=71O<BYH(&(O<W)C+VEN8VQU9&4O
M97AE8W5T;W(O97AE8W5T;W(N: II;F1E>" P-S5B8F4X+BXS869D9CDX(#$P
M,#8T- HM+2T@82]S<F,O:6YC;'5D92]E>&5C=71O<B]E>&5C=71O<BYH"BLK
M*R!B+W-R8R]I;F-L=61E+V5X96-U=&]R+V5X96-U=&]R+F@*0$ @+3$X."PV
M("LQ.#@L-R! 0"!E>'1E<FX@=F]I9"!);FET4F5S=6QT4F5L26YF;RA297-U
M;'1296Q);F9O("IR97-U;'1296Q);F9O+ H@"0D)"2 @:6YT(&EN<W1R=6UE
M;G1?;W!T:6]N<RD["B!E>'1E<FX@4F5S=6QT4F5L26YF;R J17AE8T=E=%1R
M:6=G97)297-U;'1296PH15-T871E("IE<W1A=&4L($]I9"!R96QI9"D["B!E
M>'1E<FX@8F]O;"!%>&5C0V]N=&5X=$9O<F-E<T]I9',H4&QA;E-T871E("IP
M;&%N<W1A=&4L(&)O;VP@*FAA<V]I9',I.PHK97AT97)N('9O:60@17AE8TYO
M=$YU;&Q#:&5C:RA296QA=&EO;B!R96PL(%1U<&QE5&%B;&53;&]T("IS;&]T
M*3L*(&5X=&5R;B!V;VED($5X96-#;VYS=')A:6YT<RA297-U;'1296Q);F9O
M("IR97-U;'1296Q);F9O+ H@"0D)"51U<&QE5&%B;&53;&]T("IS;&]T+"!%
M4W1A=&4@*F5S=&%T92D["B!E>'1E<FX@17AE8U)O=TUA<FL@*D5X96-&:6YD
D4F]W36%R:RA%4W1A=&4@*F5S=&%T92P@26YD97@@<G1I*3L*
`
end