I tried to review this patch. It seems to me its implementation is reasonable and enough simple. All the works of this patch is pick-up "force_not_null" option from pg_attribute.attfdwoptions and transform its data structure into suitable form to the existing BeginCopyFrom(). So, I'd almost like to mark this patch "Ready for Committer".
Here are only two points I'd like to comment on. + 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)))); The tuple should be always found unless we have any bugs that makes mismatch between pg_class.relnatts and actual number of attributes. So, it is a case to use elog(), instead of ereport() with error code. One other point is diffset of regression test, when I run `make check -C contrib/file_fdw'. Do we have something changed corresponding to COPY TO/FROM statement since 8th-August to now? *** /home/kaigai/repo/sepgsql/contrib/file_fdw/expected/file_fdw.out 2011-09-04 20:36:23.670981921 +0200 --- /home/kaigai/repo/sepgsql/contrib/file_fdw/results/file_fdw.out 2011-09-04 20:36:51.202989681 +0200 *************** *** 118,126 **** word1 | word2 -------+------- 123 | 123 ABC | ABC NULL | - abc | abc (4 rows) -- basic query tests --- 118,126 ---- word1 | word2 -------+------- 123 | 123 + abc | abc ABC | ABC NULL | (4 rows) -- basic query tests ====================================================================== Thanks, 2011年8月8日9:14 Shigeru Hanada <shigeru.han...@gmail.com>: > 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 > > > -- > Sent via pgsql-hackers mailing list (firstname.lastname@example.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- KaiGai Kohei <kai...@kaigai.gr.jp> -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers