Hi,
I propose to support force_not_null option for file_fdw too.
In 9.1 development cycle, file_fdw had been implemented with exported
COPY FROM routines, but only force_not_null option has not been　
supported yet.
Originally, in COPY FROM, force_not_null is specified as a list of
column which is not matched against null-string. Now per-column FDW
option is available, so I implemented force_not_null options as boolean
value for each column to avoid parsing column list in file_fdw.
True means that the column is not matched against null-string, and it's
equivalent to specify the column's name in force_not_null option of COPY
FROM. Default value is false.
The patch includes changes for code, document and regression tests. Any
comments/questions are welcome.
Regards,
--
Shigeru Hanada
diff --git a/contrib/file_fdw/data/text.csv b/contrib/file_fdw/data/text.csv
index ...bd4c327 .
*** a/contrib/file_fdw/data/text.csv
--- b/contrib/file_fdw/data/text.csv
***************
*** 0 ****
--- 1,4 ----
+ 123,123
+ abc,abc
+ NULL,NULL
+ ABC,ABC
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 224e74f..e846176 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 23,30 ****
--- 23,32 ----
#include "foreign/fdwapi.h"
#include "foreign/foreign.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "optimizer/cost.h"
#include "utils/rel.h"
+ #include "utils/syscache.h"
PG_MODULE_MAGIC;
*************** static struct FileFdwOption valid_option
*** 57,72 ****
{"escape", ForeignTableRelationId},
{"null", ForeignTableRelationId},
{"encoding", ForeignTableRelationId},
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
- /*
- * force_not_null is not supported by file_fdw. It would need a parser
- * for list of columns, not to mention a way to check the column list
- * against the table.
- */
/* Sentinel */
{NULL, InvalidOid}
--- 59,70 ----
{"escape", ForeignTableRelationId},
{"null", ForeignTableRelationId},
{"encoding", ForeignTableRelationId},
+ {"force_not_null", AttributeRelationId}, /* specified as boolean
value */
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
/* Sentinel */
{NULL, InvalidOid}
*************** static void fileGetOptions(Oid foreignta
*** 112,117 ****
--- 110,116 ----
static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
const char *filename,
Cost *startup_cost, Cost *total_cost);
+ static List * get_force_not_null(Oid relid);
/*
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 145,150 ****
--- 144,150 ----
List *options_list = untransformRelOptions(PG_GETARG_DATUM(0));
Oid catalog = PG_GETARG_OID(1);
char *filename = NULL;
+ char *force_not_null = NULL;
List *other_options = NIL;
ListCell *cell;
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 198,204 ****
buf.data)));
}
! /* Separate out filename, since ProcessCopyOptions won't allow
it */
if (strcmp(def->defname, "filename") == 0)
{
if (filename)
--- 198,207 ----
buf.data)));
}
! /*
! * Separate out filename and force_not_null, since
ProcessCopyOptions
! * won't allow them.
! */
if (strcmp(def->defname, "filename") == 0)
{
if (filename)
*************** file_fdw_validator(PG_FUNCTION_ARGS)
*** 207,212 ****
--- 210,229 ----
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 = defGetString(def);
+ if (strcmp(force_not_null, "true") != 0 &&
+ strcmp(force_not_null, "false") != 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("force_not_null must be
true or false")));
+ }
else
other_options = lappend(other_options, def);
}
*************** is_valid_option(const char *option, Oid
*** 245,254 ****
--- 262,356 ----
}
/*
+ * Retrieve per-column generic options from pg_attribute and construct a list
+ * of column names for "force_not_null".
+ */
+ static List *
+ get_force_not_null(Oid relid)
+ {
+ Relation rel;
+ TupleDesc tupleDesc;
+ AttrNumber natts;
+ AttrNumber attnum;
+ List *columns = NIL;
+
+ rel = heap_open(relid, AccessShareLock);
+ tupleDesc = RelationGetDescr(rel);
+ natts = tupleDesc->natts;
+
+ /* Retrieve FDW options from every user-defined attributes. */
+ for (attnum = 1; attnum < natts; attnum++)
+ {
+ HeapTuple tuple;
+ Form_pg_attribute attr;
+ Datum datum;
+ bool isnull;
+ List *options;
+ ListCell *cell;
+
+
+ tuple = SearchSysCache2(ATTNUM,
+
RelationGetRelid(rel),
+
Int16GetDatum(attnum));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("cache lookup failed for
attribute %d of relation %u",
+ attnum,
RelationGetRelid(rel))));
+ attr = (Form_pg_attribute) GETSTRUCT(tuple);
+
+ /* Skip dropped attributes. */
+ if (attr->attisdropped)
+ {
+ ReleaseSysCache(tuple);
+ continue;
+ }
+
+ datum = SysCacheGetAttr(ATTNUM,
+ tuple,
+
Anum_pg_attribute_attfdwoptions,
+ &isnull);
+ if (isnull)
+ datum = PointerGetDatum(NULL);
+ options = untransformRelOptions(datum);
+
+ /*
+ * Find force_not_null option and append attname to the list if
+ * the value was true.
+ */
+ foreach (cell, options)
+ {
+ DefElem *def = (DefElem *) lfirst(cell);
+ const char *value = defGetString(def);
+
+ if (strcmp(def->defname, "force_not_null") == 0 &&
+ strcmp(value, "true") == 0)
+ {
+ columns = lappend(columns,
makeString(NameStr(attr->attname)));
+ elog(DEBUG1, "%s: force_not_null",
NameStr(attr->attname));
+ }
+
+ }
+
+ ReleaseSysCache(tuple);
+ }
+
+ heap_close(rel, AccessShareLock);
+
+ /* Return DefElem only when any column is set "force_not_null=true". */
+ if (columns != NIL)
+ return list_make1(makeDefElem("force_not_null", (Node *)
columns));
+ else
+ return NIL;
+ }
+
+ /*
* 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.
+ * And we must construct List of DefElem from pg_attribute.attfdwoptions for
+ * "force_not_null".
*/
static void
fileGetOptions(Oid foreigntableid,
*************** fileGetOptions(Oid foreigntableid,
*** 296,301 ****
--- 398,406 ----
prev = lc;
}
+ /* Retrieve force_not_null from pg_attribute and append it to the list.
*/
+ options = list_concat(options, get_force_not_null(foreigntableid));
+
/*
* The validator should have checked that a filename was included in the
* options, but check again, just in case.
diff --git a/contrib/file_fdw/input/file_fdw.source
b/contrib/file_fdw/input/file_fdw.source
index 1405752..5d7347f 100644
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** CREATE FOREIGN TABLE agg_bad (
*** 78,83 ****
--- 78,97 ----
) SERVER file_server
OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true',
delimiter ';', quote '@', escape '"', null '');
+ -- per-column options tests
+ ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_not_null '*');
-- ERROR
+ ALTER SERVER file_server OPTIONS (ADD force_not_null '*');
-- ERROR
+ 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
+ CREATE FOREIGN TABLE text_csv (
+ word1 text OPTIONS (force_not_null 'true'),
+ word2 text OPTIONS (force_not_null 'false')
+ ) SERVER file_server
+ OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
+ SELECT * FROM text_csv ORDER BY word1; -- ERROR
+ ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
+ SELECT * FROM text_csv ORDER BY word1;
+
-- 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
index 6dd2653..cf2cba5 100644
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** CREATE FOREIGN TABLE agg_bad (
*** 93,98 ****
--- 93,128 ----
b float4
) SERVER file_server
OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true',
delimiter ';', quote '@', escape '"', null '');
+ -- per-column options tests
+ ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (ADD force_not_null '*');
-- ERROR
+ ERROR: invalid option "force_not_null"
+ HINT: Valid options in this context are:
+ ALTER SERVER file_server OPTIONS (ADD force_not_null '*');
-- ERROR
+ ERROR: invalid option "force_not_null"
+ HINT: Valid options in this context are:
+ CREATE USER MAPPING FOR public SERVER file_server OPTIONS (force_not_null
'*'); -- ERROR
+ 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
+ CREATE FOREIGN TABLE text_csv (
+ word1 text OPTIONS (force_not_null 'true'),
+ word2 text OPTIONS (force_not_null 'false')
+ ) SERVER file_server
+ OPTIONS (format 'text', filename '@abs_srcdir@/data/text.csv', null 'NULL');
+ SELECT * FROM text_csv ORDER BY word1; -- ERROR
+ ERROR: COPY force not null available only in CSV mode
+ ALTER FOREIGN TABLE text_csv OPTIONS (SET format 'csv');
+ SELECT * FROM text_csv ORDER BY word1;
+ word1 | word2
+ -------+-------
+ 123 | 123
+ ABC | ABC
+ NULL |
+ abc | abc
+ (4 rows)
+
-- basic query tests
SELECT * FROM agg_text WHERE b > 10.0 ORDER BY a;
a | b
*************** SET ROLE file_fdw_superuser;
*** 216,222 ****
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 7 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
--- 246,252 ----
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 8 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
*************** drop cascades to user mapping for no_pri
*** 224,227 ****
--- 254,258 ----
drop cascades to foreign table agg_text
drop cascades to foreign table agg_csv
drop cascades to foreign table agg_bad
+ drop cascades to foreign table text_csv
DROP ROLE file_fdw_superuser, file_fdw_user, no_priv_user;
diff --git a/doc/src/sgml/file-fdw.sgml b/doc/src/sgml/file-fdw.sgml
index 8497d9a..5b35451 100644
*** a/doc/src/sgml/file-fdw.sgml
--- b/doc/src/sgml/file-fdw.sgml
***************
*** 111,124 ****
</variablelist>
<para>
! <command>COPY</>'s <literal>OIDS</literal>, <literal>FORCE_QUOTE</literal>,
! and <literal>FORCE_NOT_NULL</literal> options are currently not supported by
<literal>file_fdw</>.
</para>
<para>
! These options can only be specified for a foreign table, not in the
! options of the <literal>file_fdw</> foreign-data wrapper, nor in the
options of a server or user mapping using the wrapper.
</para>
--- 111,147 ----
</variablelist>
<para>
! A column of a foreign table created using this wrapper can have the
! following options:
! </para>
!
! <variablelist>
!
! <varlistentry>
! <term><literal>force_not_null</literal></term>
!
! <listitem>
! <para>
! Specifies whether values for the column shouldn't been matched against
! the null string. Acceptable values are <literal>true</> for no matching,
! and <literal>false</> for matching (case sensitive).
! <literal>true</> is same as specifying the column in <command>COPY</>'s
! <literal>FORCE_NOT_NULL</literal> option.
! </para>
! </listitem>
! </varlistentry>
!
! </variablelist>
!
! <para>
! <command>COPY</>'s <literal>OIDS</literal> and
! <literal>FORCE_QUOTE</literal> options are currently not supported by
<literal>file_fdw</>.
</para>
<para>
! These options can only be specified for a foreign table or its columns, not
! in the options of the <literal>file_fdw</> foreign-data wrapper, nor in the
options of a server or user mapping using the wrapper.
</para>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
