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

Re: force_not_null option support for file_fdw

From: Kohei Kaigai <Kohei(dot)Kaigai(at)EMEA(dot)NEC(dot)COM>
To: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PostgreSQL-development<pgsql-hackers(at)postgresql(dot)org>
Subject: Re: force_not_null option support for file_fdw
Date: 2011-09-08 15:47:20
Message-ID: D0C1A1F8BF513F469926E6C71461D9EC0577D7@EX10MBX02.EU.NEC.COM (view raw or flat)
Thread:
Lists: pgsql-hackers
Hi Hanada-san.

> ISTM that your results are reasonable for each collation setting.
> Former ordering is same as C locale, and in latter case alphabetical order has priority over case
> distinctions.  Do you mean that ordering used in file_fdw is affected from something unexpected, without
> collation or locale setting?
> 
> BTW, I found a thread which is related to this issue.
>   http://archives.postgresql.org/pgsql-hackers/2011-09/msg00130.php
> 
> I changed the test data so that it uses only upper case alphabets, because case distinction is not
> important for that test.  I also removed digits to avoid test failure in some locales which sort
> alphabets before digits.
>
OK, Thanks for this clarification. This change of regression test case seems to me reasonable
to avoid unnecessary false-positive.

I found one other point to be fixed:
On get_force_not_null(), it makes a list of attribute names with force_not_null option.

+       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));
+           }
+
+       }

makeString() does not copy the supplied string itself, so it is not preferable to reference
NameStr(attr->attname) across ReleaseSysCache().
I'd like to suggest to supply a copied attname using pstrdup for makeString

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei <kohei(dot)kaigai(at)emea(dot)nec(dot)com>


> -----Original Message-----
> From: Shigeru Hanada [mailto:shigeru(dot)hanada(at)gmail(dot)com]
> Sent: 8. September 2011 06:19
> To: Kohei Kaigai
> Cc: Kohei KaiGai; PostgreSQL-development
> Subject: Re: [HACKERS] force_not_null option support for file_fdw
> 
> (2011/09/05 22:05), Kohei Kaigai wrote:
> >> 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?
> >>
> > It is not set in my environment.
> >
> > I checked the behavior of ORDER BY when we set a collation on the regular relation, not a foreign
> table.
> > Do we hit something other unexpected bug in collation here?
> >
> > postgres=# CREATE TABLE t1 (word1 text); CREATE TABLE postgres=#
> > INSERT INTO t1 VALUES ('ABC'),('abc'),('123'),('NULL'); INSERT 0 4
> > postgres=# ALTER TABLE t1 ALTER word1 TYPE text COLLATE "ja_JP.utf8";
> > ALTER TABLE postgres=# SELECT * FROM t1 ORDER BY word1;
> >   word1
> > -------
> >   123
> >   ABC
> >   NULL
> >   abc
> > (4 rows)
> >
> > postgres=# ALTER TABLE t1 ALTER word1 TYPE text COLLATE "en_US.utf8";
> > ALTER TABLE postgres=# SELECT * FROM t1 ORDER BY word1;
> >   word1
> > -------
> >   123
> >   abc
> >   ABC
> >   NULL
> > (4 rows)
> 
> Thanks for the checking.  FYI, I mainly use Fedora 15 box with Japanese environment for my development.
> 
> ISTM that your results are reasonable for each collation setting.
> Former ordering is same as C locale, and in latter case alphabetical order has priority over case
> distinctions.  Do you mean that ordering used in file_fdw is affected from something unexpected, without
> collation or locale setting?
> 
> BTW, I found a thread which is related to this issue.
>   http://archives.postgresql.org/pgsql-hackers/2011-09/msg00130.php
> 
> I changed the test data so that it uses only upper case alphabets, because case distinction is not
> important for that test.  I also removed digits to avoid test failure in some locales which sort
> alphabets before digits.
> 
> Regards,
> --
> Shigeru Hanada
> 
> 
>  Click
> https://www.mailcontrol.com/sr/fB6Wmr8zmMzTndxI!oX7Uo9cpkuWnNqkEgc!P6cHvBhGb4EkL1te5Ky38yYzoE4Mra
> 3ljAyIpUlPbv5+FCDqIw==  to report this email as spam.

In response to

Responses

pgsql-hackers by date

Next:From: Heikki LinnakangasDate: 2011-09-08 16:35:37
Subject: Fast GiST index build - further improvements
Previous:From: Robert HaasDate: 2011-09-08 15:46:36
Subject: Re: concurrent snapshots

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