(2011/07/04 10:17), Shigeru Hanada wrote:
> (2011/07/03 18:50), Kohei KaiGai wrote:
>> I checked the per-column generic option patch.
>> Right now, I have nothing to comment on anymore.
>> So, it should be reviewed by committers.
>
> Thanks for the review!.
I would like to propose adding force_not_null support to file_fdw, as
the first use case of per-column FDW option. Attached patch, which
assumes that per_column_option_v3.patch has been applied, implements
force_not_null option as per-column FDW option.
Overview
========
This option is originally supported by COPY FROM command, so I think
it's reasonable to support it in file_fdw too. It would provides more
flexible parsing capability. In fact, this option has been supported
by the internal routines which are shared with COPY FROM, but currently
we don't have any way to specify it.
Difference between COPY
=======================
For COPY FROM, FORCE_NOT_NULL is specified as a list of column names
('*' is not allowed). For file_fdw, per-table FDW option can be used
like other options, but then file_fdw needs parser which can identify
valid column. I think it's too much work, so I prefer per-column FDW
option which accepts boolean value string. The value 'true' means that
the column doesn't be matched against NULL string, same as ones listed
for COPY FROM.
Example:
If you have created a foreign table with:
CREATE FOREIGN TABLE foo (
c1 int OPTIONS (force_not_null 'false'),
c2 text OPTIONS (force_not_null 'true')
) SERVER file OPTIONS (file '/path/to/file', format 'csv', null '');
values which are read from the file for c1 are matched against
null-representation-string '', but values for c2 are NOT. Empty strings
for c2 are stored as empty strings; they don't treated as NULL value.
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 466c015..caf9c86 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 23,29 ****
--- 23,31 ----
#include "foreign/fdwapi.h"
#include "foreign/foreign.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "optimizer/cost.h"
+ #include "utils/syscache.h"
PG_MODULE_MAGIC;
*************** static struct FileFdwOption valid_option
*** 56,71 ****
{"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}
--- 58,69 ----
{"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
*** 111,116 ****
--- 109,115 ----
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)
*** 144,149 ****
--- 143,149 ----
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)
*** 197,203 ****
buf.data)));
}
! /* Separate out filename, since ProcessCopyOptions won't allow
it */
if (strcmp(def->defname, "filename") == 0)
{
if (filename)
--- 197,206 ----
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)
*** 206,211 ****
--- 209,228 ----
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
*** 236,245 ****
--- 253,347 ----
}
/*
+ * 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 DefElemn 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,
*** 286,291 ****
--- 388,398 ----
}
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 filename is required optiono. */
if (*filename == NULL)
ereport(ERROR,
(errcode(ERRCODE_FDW_UNABLE_TO_CREATE_REPLY),
diff --git a/contrib/file_fdw/input/file_fdw.source
b/contrib/file_fdw/input/file_fdw.source
index 9ff7235..51e8ff0 100644
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** CREATE FOREIGN TABLE agg_bad (
*** 77,82 ****
--- 77,96 ----
) 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 2ba36c9..e4c4700 100644
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** CREATE FOREIGN TABLE agg_bad (
*** 91,96 ****
--- 91,126 ----
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;
*** 214,220 ****
-- 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
--- 244,250 ----
-- 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
*** 222,225 ****
--- 252,256 ----
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..776aa9a 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 specifing 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
