From 86a3f2d492f19da1f4be8ba099747ac5c83c43bb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 16 Sep 2011 16:35:51 -0400 Subject: [PATCH] Add FORCE_NOT_NULL support to the file_fdw foreign data wrapper. This is implemented as a per-column boolean option, rather than trying to match COPY's convention of a single option listing the column names. Shigeru Hanada, reviewed by KaiGai Kohei --- contrib/file_fdw/data/text.csv | 4 + contrib/file_fdw/file_fdw.c | 112 ++++++++++++++++++++++-- contrib/file_fdw/input/file_fdw.source | 16 ++++ contrib/file_fdw/output/file_fdw.source | 34 ++++++- doc/src/sgml/file-fdw.sgml | 31 ++++++- 5 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 contrib/file_fdw/data/text.csv diff --git a/contrib/file_fdw/data/text.csv b/contrib/file_fdw/data/text.csv new file mode 100644 index 0000000000..ed348a9e84 --- /dev/null +++ b/contrib/file_fdw/data/text.csv @@ -0,0 +1,4 @@ +AAA,aaa +XYZ,xyz +NULL,NULL +ABC,abc diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 224e74ff32..1cf3b3c4e0 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -23,8 +23,10 @@ #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; @@ -40,6 +42,8 @@ struct FileFdwOption /* * 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. @@ -57,17 +61,12 @@ static struct FileFdwOption valid_options[] = { {"escape", ForeignTableRelationId}, {"null", ForeignTableRelationId}, {"encoding", ForeignTableRelationId}, + {"force_not_null", AttributeRelationId}, /* * 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} }; @@ -109,6 +108,7 @@ static void fileEndForeignScan(ForeignScanState *node); static bool is_valid_option(const char *option, Oid context); static void fileGetOptions(Oid foreigntableid, char **filename, List **other_options); +static List *get_file_fdw_attribute_options(Oid relid); static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel, const char *filename, Cost *startup_cost, Cost *total_cost); @@ -145,6 +145,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 *force_not_null = NULL; List *other_options = NIL; ListCell *cell; @@ -198,7 +199,11 @@ file_fdw_validator(PG_FUNCTION_ARGS) buf.data))); } - /* Separate out filename, since ProcessCopyOptions won't allow it */ + /* + * 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) @@ -207,6 +212,16 @@ file_fdw_validator(PG_FUNCTION_ARGS) 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); } @@ -277,6 +292,7 @@ fileGetOptions(Oid foreigntableid, options = list_concat(options, wrapper->options); options = list_concat(options, server->options); options = list_concat(options, table->options); + options = list_concat(options, get_file_fdw_attribute_options(foreigntableid)); /* * Separate out the filename. @@ -306,6 +322,88 @@ fileGetOptions(Oid foreigntableid, *other_options = options; } +/* + * 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) +{ + Relation rel; + TupleDesc tupleDesc; + AttrNumber natts; + AttrNumber attnum; + List *fnncolumns = NIL; + + rel = heap_open(relid, AccessShareLock); + tupleDesc = RelationGetDescr(rel); + natts = tupleDesc->natts; + + /* Retrieve FDW options for all user-defined attributes. */ + for (attnum = 1; attnum <= natts; attnum++) + { + HeapTuple tuple; + Form_pg_attribute attr; + Datum datum; + bool isnull; + + /* Skip dropped attributes. */ + if (tupleDesc->attrs[attnum - 1]->attisdropped) + continue; + + /* + * We need the whole pg_attribute tuple not just what is in the + * tupleDesc, so must do a catalog lookup. + */ + tuple = SearchSysCache2(ATTNUM, + RelationGetRelid(rel), + Int16GetDatum(attnum)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for attribute %d of relation %u", + attnum, RelationGetRelid(rel)); + attr = (Form_pg_attribute) GETSTRUCT(tuple); + + datum = SysCacheGetAttr(ATTNUM, + tuple, + Anum_pg_attribute_attfdwoptions, + &isnull); + if (!isnull) + { + List *options = untransformRelOptions(datum); + ListCell *lc; + + foreach(lc, options) + { + DefElem *def = (DefElem *) lfirst(lc); + + if (strcmp(def->defname, "force_not_null") == 0) + { + if (defGetBoolean(def)) + { + char *attname = pstrdup(NameStr(attr->attname)); + + fnncolumns = lappend(fnncolumns, makeString(attname)); + } + } + /* maybe in future handle other options here */ + } + } + + ReleaseSysCache(tuple); + } + + 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; +} + /* * filePlanForeignScan * Create a FdwPlan for a scan on the foreign table diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source index 1405752819..8e3d553f90 100644 --- a/contrib/file_fdw/input/file_fdw.source +++ b/contrib/file_fdw/input/file_fdw.source @@ -78,6 +78,22 @@ CREATE FOREIGN TABLE agg_bad ( ) SERVER file_server OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null ''); +-- 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: +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 + -- 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 6dd2653d0a..84f07501a0 100644 --- a/contrib/file_fdw/output/file_fdw.source +++ b/contrib/file_fdw/output/file_fdw.source @@ -93,6 +93,37 @@ CREATE FOREIGN TABLE agg_bad ( b float4 ) SERVER file_server OPTIONS (format 'csv', filename '@abs_srcdir@/data/agg.bad', header 'true', delimiter ';', quote '@', escape '"', null ''); +-- 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 +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 -- basic query tests SELECT * FROM agg_text WHERE b > 10.0 ORDER BY a; a | b @@ -216,7 +247,7 @@ SET ROLE file_fdw_superuser; -- cleanup RESET ROLE; DROP EXTENSION file_fdw CASCADE; -NOTICE: drop cascades to 7 other objects +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 @@ -224,4 +255,5 @@ drop cascades to user mapping for no_priv_user 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 8497d9a45f..dd712e9263 100644 --- a/doc/src/sgml/file-fdw.sgml +++ b/doc/src/sgml/file-fdw.sgml @@ -111,14 +111,37 @@ - COPY's OIDS, FORCE_QUOTE, - and FORCE_NOT_NULL options are currently not supported by + A column of a foreign table created using this wrapper can have the + following options: + + + + + + force_not_null + + + + This is a boolean option. If true, it specifies that values of the + column should not be matched against the null string (that is, the + file-level null option). This has the same effect + as listing the column in COPY's + FORCE_NOT_NULL option. + + + + + + + + COPY's OIDS and + FORCE_QUOTE options are currently not supported by file_fdw. - These options can only be specified for a foreign table, not in the - options of the file_fdw foreign-data wrapper, nor in the + These options can only be specified for a foreign table or its columns, not + in the options of the file_fdw foreign-data wrapper, nor in the options of a server or user mapping using the wrapper.