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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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

Browse pgsql-hackers by date

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