pgsql: Add citext_pattern_ops for citext contrib module

Started by Andrew Dunstanover 8 years ago8 messages
#1Andrew Dunstan
andrew@dunslane.net

Add citext_pattern_ops for citext contrib module

This is similar to text_pattern_ops.

Alexey Chernyshov, reviewed by Jacob Champion.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/f2464997644c64b5dec93ab3c08305f48bfe14f1

Modified Files
--------------
contrib/citext/citext--1.4--1.5.sql | 74 +++++++
contrib/citext/citext.c | 121 ++++++++++++
contrib/citext/expected/citext.out | 370 +++++++++++++++++++++++++++++++++++
contrib/citext/expected/citext_1.out | 370 +++++++++++++++++++++++++++++++++++
contrib/citext/sql/citext.sql | 78 ++++++++
5 files changed, 1013 insertions(+)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Andrew Dunstan (#1)
Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

On 09/19/2017 08:35 AM, Andrew Dunstan wrote:

Add citext_pattern_ops for citext contrib module

This is similar to text_pattern_ops.

This seems to have upset a number or animals in the buildfarm.

I could create a third output file, but I am seriously questioning the
point of all this, if we are prepared to accept any result from these
functions and operators, depending on locale. Maybe it would be better
simply to test that the result is not null and have done with it. Thoughts?

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Simon Riggs
simon@2ndquadrant.com
In reply to: Andrew Dunstan (#2)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

On 19 September 2017 at 15:22, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:

On 09/19/2017 08:35 AM, Andrew Dunstan wrote:

Add citext_pattern_ops for citext contrib module

This is similar to text_pattern_ops.

This seems to have upset a number or animals in the buildfarm.

I could create a third output file, but I am seriously questioning the
point of all this, if we are prepared to accept any result from these
functions and operators, depending on locale. Maybe it would be better
simply to test that the result is not null and have done with it. Thoughts?

It makes sense to have a fully detailed output in at least one
parameter setting.

How about use the new psql feature for \if to skip tests if the local
is different to the one for which we have detailed test results?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#2)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

This seems to have upset a number or animals in the buildfarm.

Looks like all the ones that are testing in en_US locale.

I could create a third output file, but I am seriously questioning the
point of all this,

What locale did you use to create citext_1.out? We've never before
needed more than one output file for non-C locales.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#2)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

This seems to have upset a number or animals in the buildfarm.

Actually, after looking closer, my advice is just to drop the new
test cases involving accented letters. It surprises me not in the
least that those would have nonportable behavior: probably, some
locales will case-fold them and some won't.

(This does open up some questions about whether the opclass will
ever be of use for Alexey's original purpose :-()

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Tom Lane (#5)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

On 09/19/2017 11:11 AM, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

This seems to have upset a number or animals in the buildfarm.

Actually, after looking closer, my advice is just to drop the new
test cases involving accented letters. It surprises me not in the
least that those would have nonportable behavior: probably, some
locales will case-fold them and some won't.

(This does open up some questions about whether the opclass will
ever be of use for Alexey's original purpose :-()

Actually, it now looks to me like the problem is we forgot to tell
postgres that this data is in utf8. The problem appears to resolve (at
least on my CentOS test box, where I reproduced the buildfarm error) if
I add

set client_encoding = 'utf8';

to the sql file.

Of course UTF8 bytes interpreted as LATIN1 will give results that are
... interesting.

So let's try that first and see if it make the buildfarm go green. Maybe
there's hope for Alexey's purpose after all.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#6)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 09/19/2017 11:11 AM, Tom Lane wrote:

Actually, after looking closer, my advice is just to drop the new
test cases involving accented letters. It surprises me not in the
least that those would have nonportable behavior: probably, some
locales will case-fold them and some won't.

Actually, it now looks to me like the problem is we forgot to tell
postgres that this data is in utf8. The problem appears to resolve (at
least on my CentOS test box, where I reproduced the buildfarm error) if
I add
set client_encoding = 'utf8';
to the sql file.

That took care of one source of problems, but I wouldn't have expected it
to resolve all the problems ... and it didn't.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Andrew Dunstan (#6)
Re: Re: [COMMITTERS] pgsql: Add citext_pattern_ops for citext contrib module

On 09/19/2017 02:47 PM, Andrew Dunstan wrote:

On 09/19/2017 11:11 AM, Tom Lane wrote:

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

This seems to have upset a number or animals in the buildfarm.

Actually, after looking closer, my advice is just to drop the new
test cases involving accented letters. It surprises me not in the
least that those would have nonportable behavior: probably, some
locales will case-fold them and some won't.

(This does open up some questions about whether the opclass will
ever be of use for Alexey's original purpose :-()

Actually, it now looks to me like the problem is we forgot to tell
postgres that this data is in utf8. The problem appears to resolve (at
least on my CentOS test box, where I reproduced the buildfarm error) if
I add

set client_encoding = 'utf8';

to the sql file.

Of course UTF8 bytes interpreted as LATIN1 will give results that are
... interesting.

So let's try that first and see if it make the buildfarm go green. Maybe
there's hope for Alexey's purpose after all.

*sigh* That worked in a couple of instances but crashed and burned
elsewhere. I'll just disable the multi-byte tests as Tom suggests.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers