Skip site navigation (1) Skip section navigation (2)

Re: force_not_null option support for file_fdw

From: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: force_not_null option support for file_fdw
Date: 2011-09-05 05:55:55
Message-ID: 4E64646B.9050009@gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
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


Attachment: force_not_null_v3.patch
Description: text/plain (12.0 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Oleg BartunovDate: 2011-09-05 06:37:05
Subject: Re: WIP: SP-GiST, Space-Partitioned GiST
Previous:From: Tom LaneDate: 2011-09-04 22:31:38
Subject: Re: limit in subquery causes poor selectivity estimation

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group