Thread

Re: force_not_null option support for file_fdw

Shigeru Hanada <shigeru.hanada@gmail.com> — 2011-09-05T05:55:55Z

Thanks for the review.

(2011/09/05 3:55), Kohei KaiGai wrote:
> 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.

Oh, I've missed that other similar errors use elog()...
Fixed.

> 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?

I don't know about such change, and src/backend/command/copy.c has not
been touched since Feb 23.

> *** /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
> 
> ======================================================================

In my usual environment that test passed, but finally I've reproduced
the failure with setting $LC_COLLATE to "es_ES.UTF-8".  Do you have set
any $LC_COLLATE in your test environment?

Regards,
-- 
Shigeru Hanada