file_fdw_notnull_v3.patch

application/octet-stream

Filename: file_fdw_notnull_v3.patch
Type: application/octet-stream
Part: 0
Message: Re: not null validation option in contrib/file_fdw

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v3
File+
contrib/file_fdw/file_fdw.c 70 17
contrib/file_fdw/output/file_fdw.source 1 1
doc/src/sgml/file-fdw.sgml 10 0
src/backend/executor/execMain.c 23 16
src/include/executor/executor.h 1 0
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);