public inbox for [email protected]  
help / color / mirror / Atom feed
strcmp() tie-breaker for identical ICU-collated strings
33+ messages / 9 participants
[nested] [flat]

* strcmp() tie-breaker for identical ICU-collated strings
@ 2017-06-01 18:58 Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Amit Khandekar @ 2017-06-01 18:58 UTC (permalink / raw)
  To: pgsql-hackers

While comparing two text strings using varstr_cmp(), if *strcoll*()
call returns 0, we do strcmp() tie-breaker to do binary comparison,
because strcoll() can return 0 for non-identical strings :

varstr_cmp()
{
...
/*
* In some locales strcoll() can claim that nonidentical strings are
* equal.  Believing that would be bad news for a number of reasons,
* so we follow Perl's lead and sort "equal" strings according to
* strcmp().
*/
if (result == 0)
result = strcmp(a1p, a2p);
...
}

But is this supposed to apply for ICU collations as well ? If
collation provider is icu, the comparison is done using
ucol_strcoll*(). I suspect that ucol_strcoll*() intentionally returns
some characters as being identical, so doing strcmp() may not make
sense.

For e.g. , if the below two characters are compared using
ucol_strcollUTF8(), it returns 0, meaning the strings are identical :
Greek Oxia : UTF-16 encoding : 0x1FFD
(http://www.fileformat.info/info/unicode/char/1ffd/index.htm)
Greek Tonos : UTF-16 encoding : 0x0384
(http://www.fileformat.info/info/unicode/char/0384/index.htm)

The characters are displayed like this :
postgres=# select (U&'\+001FFD') , (U&'\+000384') collate ucatest;
 ?column? | ?column?
----------+----------
 ´        | ΄
(Although this example has similar looking characters, this might not
be a factor behind treating them equal)

Now since ucol_strcoll*() returns 0, these strings are always compared
using strcmp(), so 1FFD > 0384 returns true :

create collation ucatest (locale = 'en_US.UTF8', provider = 'icu');

postgres=# select (U&'\+001FFD') > (U&'\+000384') collate ucatest;
 ?column?
----------
 t

Whereas, if strcmp() is skipped for ICU collations :
if (result == 0 && !(mylocale && mylocale->provider == COLLPROVIDER_ICU))
   result = strcmp(a1p, a2p);

... then the comparison using ICU collation tells they are identical strings :

postgres=# select (U&'\+001FFD') > (U&'\+000384') collate ucatest;
 ?column?
----------
 f
(1 row)

postgres=# select (U&'\+001FFD') < (U&'\+000384') collate ucatest;
 ?column?
----------
 f
(1 row)

postgres=# select (U&'\+001FFD') <= (U&'\+000384') collate ucatest;
 ?column?
----------
 t


Now I have verified that strcoll() returns true for 1FFD > 0384. So,
it looks like ICU API function ucol_strcoll() returns false by
intention. That's the reason I feel like the
strcmp-if-strtoll-returns-0 thing might not be applicable for ICU. But
I may be wrong, please correct me if I may be missing something.


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
@ 2017-06-01 21:24 ` Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Thomas Munro @ 2017-06-01 21:24 UTC (permalink / raw)
  To: Amit Khandekar <[email protected]>; +Cc: pgsql-hackers

On Fri, Jun 2, 2017 at 6:58 AM, Amit Khandekar <[email protected]> wrote:
> While comparing two text strings using varstr_cmp(), if *strcoll*()
> call returns 0, we do strcmp() tie-breaker to do binary comparison,
> because strcoll() can return 0 for non-identical strings :
>
> varstr_cmp()
> {
> ...
> /*
> * In some locales strcoll() can claim that nonidentical strings are
> * equal.  Believing that would be bad news for a number of reasons,
> * so we follow Perl's lead and sort "equal" strings according to
> * strcmp().
> */
> if (result == 0)
> result = strcmp(a1p, a2p);
> ...
> }
>
> But is this supposed to apply for ICU collations as well ? If
> collation provider is icu, the comparison is done using
> ucol_strcoll*(). I suspect that ucol_strcoll*() intentionally returns
> some characters as being identical, so doing strcmp() may not make
> sense.
>
> For e.g. , if the below two characters are compared using
> ucol_strcollUTF8(), it returns 0, meaning the strings are identical :
> Greek Oxia : UTF-16 encoding : 0x1FFD
> (http://www.fileformat.info/info/unicode/char/1ffd/index.htm)
> Greek Tonos : UTF-16 encoding : 0x0384
> (http://www.fileformat.info/info/unicode/char/0384/index.htm)
>
> The characters are displayed like this :
> postgres=# select (U&'\+001FFD') , (U&'\+000384') collate ucatest;
>  ?column? | ?column?
> ----------+----------
>  ´        | ΄
> (Although this example has similar looking characters, this might not
> be a factor behind treating them equal)
>
> Now since ucol_strcoll*() returns 0, these strings are always compared
> using strcmp(), so 1FFD > 0384 returns true :
>
> create collation ucatest (locale = 'en_US.UTF8', provider = 'icu');
>
> postgres=# select (U&'\+001FFD') > (U&'\+000384') collate ucatest;
>  ?column?
> ----------
>  t
>
> Whereas, if strcmp() is skipped for ICU collations :
> if (result == 0 && !(mylocale && mylocale->provider == COLLPROVIDER_ICU))
>    result = strcmp(a1p, a2p);
>
> ... then the comparison using ICU collation tells they are identical strings :
>
> postgres=# select (U&'\+001FFD') > (U&'\+000384') collate ucatest;
>  ?column?
> ----------
>  f
> (1 row)
>
> postgres=# select (U&'\+001FFD') < (U&'\+000384') collate ucatest;
>  ?column?
> ----------
>  f
> (1 row)
>
> postgres=# select (U&'\+001FFD') <= (U&'\+000384') collate ucatest;
>  ?column?
> ----------
>  t
>
>
> Now I have verified that strcoll() returns true for 1FFD > 0384. So,
> it looks like ICU API function ucol_strcoll() returns false by
> intention. That's the reason I feel like the
> strcmp-if-strtoll-returns-0 thing might not be applicable for ICU. But
> I may be wrong, please correct me if I may be missing something.

I may not have had enough coffee yet, but...

Why should ICU be any different than the system provider in this
respect?  In both cases, we have a two-level comparison: first we use
the collation-aware comparison, and then as a tie breaker, we use a
binary comparison.  If we didn't do a binary comparison as a
tie-breaker, wouldn't the result be logically incompatible with the =
operator, which does a binary comparison?

Put another way, if we didn't use binary order tie-breaking, we'd have
to teach texteq to understand collations (ie be defined as not (a < b)
and not (b > a)) otherwise we'd permit contradictions like a != b and
not (a < b) and not (b > a).

-- 
Thomas Munro
http://www.enterprisedb.com


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
@ 2017-06-01 21:27   ` Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Peter Geoghegan @ 2017-06-01 21:27 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Amit Khandekar <[email protected]>; pgsql-hackers

On Thu, Jun 1, 2017 at 2:24 PM, Thomas Munro
<[email protected]> wrote:
> Why should ICU be any different than the system provider in this
> respect?  In both cases, we have a two-level comparison: first we use
> the collation-aware comparison, and then as a tie breaker, we use a
> binary comparison.  If we didn't do a binary comparison as a
> tie-breaker, wouldn't the result be logically incompatible with the =
> operator, which does a binary comparison?

I agree with that assessment.


-- 
Peter Geoghegan


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
@ 2017-06-01 21:48     ` Tom Lane <[email protected]>
  1 sibling, 0 replies; 33+ messages in thread

From: Tom Lane @ 2017-06-01 21:48 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Thomas Munro <[email protected]>; Amit Khandekar <[email protected]>; pgsql-hackers

Peter Geoghegan <[email protected]> writes:
> On Thu, Jun 1, 2017 at 2:24 PM, Thomas Munro
> <[email protected]> wrote:
>> Why should ICU be any different than the system provider in this
>> respect?  In both cases, we have a two-level comparison: first we use
>> the collation-aware comparison, and then as a tie breaker, we use a
>> binary comparison.  If we didn't do a binary comparison as a
>> tie-breaker, wouldn't the result be logically incompatible with the =
>> operator, which does a binary comparison?

> I agree with that assessment.

The critical reason why this is not optional is that if texteq were to
return true for strings that aren't bitwise identical, that breaks hashing
--- unless you can guarantee that the hash values for such strings will be
equal anyway.  That's hardly possible when we don't even know what the
collation's comparison rule is, and would likely be difficult even if
we had complete knowledge.

So no, we're not going there for ICU any more than we did for libc.

			regards, tom lane


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
@ 2017-06-01 21:48     ` Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Thomas Munro @ 2017-06-01 21:48 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Amit Khandekar <[email protected]>; pgsql-hackers

On Fri, Jun 2, 2017 at 9:27 AM, Peter Geoghegan <[email protected]> wrote:
> On Thu, Jun 1, 2017 at 2:24 PM, Thomas Munro
> <[email protected]> wrote:
>> Why should ICU be any different than the system provider in this
>> respect?  In both cases, we have a two-level comparison: first we use
>> the collation-aware comparison, and then as a tie breaker, we use a
>> binary comparison.  If we didn't do a binary comparison as a
>> tie-breaker, wouldn't the result be logically incompatible with the =
>> operator, which does a binary comparison?
>
> I agree with that assessment.

I think you *could* make a logically consistent set of operations with
no binary tie-breaker.  = could be defined in terms of strcoll and
hash could hash the output of strxfrm, but it it'd be impractical and
slow.  In order to take advantage of simple and fast = and hash, we go
the other way and teach < and > about binary order.

-- 
Thomas Munro
http://www.enterprisedb.com


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
@ 2017-06-02 17:34       ` Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Amit Khandekar @ 2017-06-02 17:34 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; pgsql-hackers

On 2 June 2017 at 03:18, Thomas Munro <[email protected]> wrote:
> On Fri, Jun 2, 2017 at 9:27 AM, Peter Geoghegan <[email protected]> wrote:
>> On Thu, Jun 1, 2017 at 2:24 PM, Thomas Munro
>> <[email protected]> wrote:
>>> Why should ICU be any different than the system provider in this
>>> respect?  In both cases, we have a two-level comparison: first we use
>>> the collation-aware comparison, and then as a tie breaker, we use a
>>> binary comparison.  If we didn't do a binary comparison as a
>>> tie-breaker, wouldn't the result be logically incompatible with the =
>>> operator, which does a binary comparison?

Ok. I was thinking we are doing the tie-breaker because specifically
strcoll_l() was unexpectedly returning 0 for some cases. Now I get it,
that we do that to be compatible with texteq().

Secondly, I was also considering if ICU especially has a way to
customize an ICU locale by setting some attributes which dictate
comparison or sorting rules for a set of characters. I mean, if there
is such customized ICU locale defined in the system, and we use that
to create PG collation, I thought we might have to strictly follow
those rules without a tie-breaker, so as to be 100% conformant to ICU.
I can't come up with an example, or may there isn't one, but , say ,
there is a locale which is supposed to sort only by lowest comparison
strength (de@strength=1 ?? ). In that case, there might be many
characters considered equal, but PG < operator or > operator would
still return true for those chars.


-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
@ 2017-06-02 18:22         ` Peter Geoghegan <[email protected]>
  2017-06-09 06:27           ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Peter Geoghegan @ 2017-06-02 18:22 UTC (permalink / raw)
  To: Amit Khandekar <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 2, 2017 at 10:34 AM, Amit Khandekar <[email protected]> wrote:
> Ok. I was thinking we are doing the tie-breaker because specifically
> strcoll_l() was unexpectedly returning 0 for some cases. Now I get it,
> that we do that to be compatible with texteq().

Both of these explanations are correct, in a way. See commit 656beff.

> Secondly, I was also considering if ICU especially has a way to
> customize an ICU locale by setting some attributes which dictate
> comparison or sorting rules for a set of characters. I mean, if there
> is such customized ICU locale defined in the system, and we use that
> to create PG collation, I thought we might have to strictly follow
> those rules without a tie-breaker, so as to be 100% conformant to ICU.
> I can't come up with an example, or may there isn't one, but , say ,
> there is a locale which is supposed to sort only by lowest comparison
> strength (de@strength=1 ?? ). In that case, there might be many
> characters considered equal, but PG < operator or > operator would
> still return true for those chars.

In the terminology of the Unicode collation algorithm, PostgreSQL
"forces deterministic comparisons" [1]. There is a lot of information
on the details of that within the UCA spec.

If we ever wanted to offer a case insensitive collation feature, then
we wouldn't necessarily have to do the equivalent of a full strxfrm()
when hashing, at least with collations controlled by ICU. Perhaps we
could instead use a collator whose UCOL_STRENGTH is only UCOL_PRIMARY
to build binary sort keys, and leave the rest to a ucol_equal() call
(within texteq()) that has the usual UCOL_STRENGTH for the underlying
PostgreSQL collation.

I don't think it would be possible to implement case insensitive
collations by using some pre-existing ICU collation that is case
insensitive. Instead, an implementation might directly vary collation
strength of any given collation to achieve case insensitivity.
PostgreSQL would know that this collation was case insensitive, so
regular collations wouldn't need to change their
behavior/implementation (to use ucol_equal() within texteq(), and so
on).

[1] http://unicode.org/reports/tr10/#Forcing_Deterministic_Comparisons
-- 
Peter Geoghegan


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
@ 2017-06-09 06:27           ` Amit Khandekar <[email protected]>
  1 sibling, 0 replies; 33+ messages in thread

From: Amit Khandekar @ 2017-06-09 06:27 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers

On 2 June 2017 at 23:52, Peter Geoghegan <[email protected]> wrote:
> On Fri, Jun 2, 2017 at 10:34 AM, Amit Khandekar <[email protected]> wrote:
>> Ok. I was thinking we are doing the tie-breaker because specifically
>> strcoll_l() was unexpectedly returning 0 for some cases. Now I get it,
>> that we do that to be compatible with texteq().
>
> Both of these explanations are correct, in a way. See commit 656beff.
>
>> Secondly, I was also considering if ICU especially has a way to
>> customize an ICU locale by setting some attributes which dictate
>> comparison or sorting rules for a set of characters. I mean, if there
>> is such customized ICU locale defined in the system, and we use that
>> to create PG collation, I thought we might have to strictly follow
>> those rules without a tie-breaker, so as to be 100% conformant to ICU.
>> I can't come up with an example, or may there isn't one, but , say ,
>> there is a locale which is supposed to sort only by lowest comparison
>> strength (de@strength=1 ?? ). In that case, there might be many
>> characters considered equal, but PG < operator or > operator would
>> still return true for those chars.
>
> In the terminology of the Unicode collation algorithm, PostgreSQL
> "forces deterministic comparisons" [1]. There is a lot of information
> on the details of that within the UCA spec.
>
> If we ever wanted to offer a case insensitive collation feature, then
> we wouldn't necessarily have to do the equivalent of a full strxfrm()
> when hashing, at least with collations controlled by ICU. Perhaps we
> could instead use a collator whose UCOL_STRENGTH is only UCOL_PRIMARY
> to build binary sort keys, and leave the rest to a ucol_equal() call
> (within texteq()) that has the usual UCOL_STRENGTH for the underlying
> PostgreSQL collation.
>
> I don't think it would be possible to implement case insensitive
> collations by using some pre-existing ICU collation that is case
> insensitive. Instead, an implementation might directly vary collation
> strength of any given collation to achieve case insensitivity.
> PostgreSQL would know that this collation was case insensitive, so
> regular collations wouldn't need to change their
> behavior/implementation (to use ucol_equal() within texteq(), and so
> on).

Ah ok. Understood, thanks.


Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
@ 2017-06-09 14:31           ` Robert Haas <[email protected]>
  2017-06-09 15:05             ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  1 sibling, 2 replies; 33+ messages in thread

From: Robert Haas @ 2017-06-09 14:31 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 2, 2017 at 2:22 PM, Peter Geoghegan <[email protected]> wrote:
> On Fri, Jun 2, 2017 at 10:34 AM, Amit Khandekar <[email protected]> wrote:
>> Ok. I was thinking we are doing the tie-breaker because specifically
>> strcoll_l() was unexpectedly returning 0 for some cases. Now I get it,
>> that we do that to be compatible with texteq().
>
> Both of these explanations are correct, in a way. See commit 656beff.

I have to admit that I'm still a little confused about what's actually
going on here.  Commit says that it "fixes inconsistent behavior under
glibc's hu_HU locale", but it doesn't say what sort of inconsistent
behavior it fixes.  It added a comment - which remains to this day -
saying this:

+         * In some locales strcoll() can claim that nonidentical strings are
+         * equal.  Believing that would be bad news for a number of reasons,
+         * so we follow Perl's lead and sort "equal" strings according to
+         * strcmp().

Again, however, the reasons why believing it would be bad news are not
enumerated.  It is merely asserted that there is more than one such
reason.

Now, it is obviously not true in general that a comparison operator
can never deem two values which are not byte-for-byte identical as
equal, because citext does exactly that (indeed, that's the point).  I
thought maybe citext could get away with it because it lacked indexing
support but, nope, it has indexing support.  Also, the in-core numeric
data type has the same property ('1.0'::numeric = '1'::numeric, but
scale() reveals that they are not byte-for-byte identical).

So, what's special about text that it can never report two
non-byte-for-byte values as equal?  And could we consider changing
that, so that users can select an ICU collator and get exactly the
behavior ICU delivers, without the extra tiebreak?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
@ 2017-06-09 15:05             ` Peter Eisentraut <[email protected]>
  1 sibling, 0 replies; 33+ messages in thread

From: Peter Eisentraut @ 2017-06-09 15:05 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; Peter Geoghegan <[email protected]>; +Cc: Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On 6/9/17 10:31, Robert Haas wrote:
> +         * In some locales strcoll() can claim that nonidentical strings are
> +         * equal.  Believing that would be bad news for a number of reasons,
> +         * so we follow Perl's lead and sort "equal" strings according to
> +         * strcmp().
> 
> Again, however, the reasons why believing it would be bad news are not
> enumerated.  It is merely asserted that there is more than one such
> reason.

I suspect that there were just issues that haven't been thought through
yet, including hashing.

More generally, the code's receptiveness to internationalization issues
is ever expanding.  Early code probably also thought that using
multibyte characters or non-C locales was bad news.  Over time, we have
worked those issues out.  This might be just be one more.

> So, what's special about text that it can never report two
> non-byte-for-byte values as equal?  And could we consider changing
> that, so that users can select an ICU collator and get exactly the
> behavior ICU delivers, without the extra tiebreak?

I don't think there is anything special.  We just need to work through
the details.

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


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
@ 2017-06-09 15:12             ` Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Tom Lane @ 2017-06-09 15:12 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

Robert Haas <[email protected]> writes:
> I have to admit that I'm still a little confused about what's actually
> going on here.  Commit says that it "fixes inconsistent behavior under
> glibc's hu_HU locale", but it doesn't say what sort of inconsistent
> behavior it fixes.

Unfortunately we were not good back then about linking commits to
list discussions, but a bit of excavation in the archives found this:

https://www.postgresql.org/message-id/[email protected]

			regards, tom lane


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
@ 2017-06-09 15:29               ` Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Peter Eisentraut @ 2017-06-09 15:29 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Robert Haas <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On 6/9/17 11:12, Tom Lane wrote:
> Robert Haas <[email protected]> writes:
>> I have to admit that I'm still a little confused about what's actually
>> going on here.  Commit says that it "fixes inconsistent behavior under
>> glibc's hu_HU locale", but it doesn't say what sort of inconsistent
>> behavior it fixes.
> 
> Unfortunately we were not good back then about linking commits to
> list discussions, but a bit of excavation in the archives found this:
> 
> https://www.postgresql.org/message-id/[email protected]

Good to know.  That just says that if we were to go with the strcoll()
result only, things would work correctly.

Again, some other details to work out.

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


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
@ 2017-06-09 15:46                 ` Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Tom Lane @ 2017-06-09 15:46 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Robert Haas <[email protected]>; Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

Peter Eisentraut <[email protected]> writes:
> On 6/9/17 11:12, Tom Lane wrote:
>> https://www.postgresql.org/message-id/[email protected]

> Good to know.  That just says that if we were to go with the strcoll()
> result only, things would work correctly.

There's still the hashing problem.

			regards, tom lane


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
@ 2017-06-09 16:17                   ` Robert Haas <[email protected]>
  2017-06-09 16:18                     ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 17:45                     ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  0 siblings, 2 replies; 33+ messages in thread

From: Robert Haas @ 2017-06-09 16:17 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 9, 2017 at 11:46 AM, Tom Lane <[email protected]> wrote:
> Peter Eisentraut <[email protected]> writes:
>> On 6/9/17 11:12, Tom Lane wrote:
>>> https://www.postgresql.org/message-id/[email protected]
>
>> Good to know.  That just says that if we were to go with the strcoll()
>> result only, things would work correctly.
>
> There's still the hashing problem.

Tom, that mailing list discussions is very illuminating.  Thanks for
digging it up.

Regarding the question of hashing, one way to support that would be if
we had some sort of canonicalization function.  IOW, suppose there
were a collation API call distill() which had the property that
strcmp(distill(X), distill(Y)) == 0 iff X and Y are considered equal
under that collation.  Then, you could define your hash function as
hash_any(distill(X)).  Alternatively, if the collation library
provided its own hashing function, that would be fine too, and
probably faster.

On the other hand, is there any rule that says we have to support
hashing?  Certainly, if we defined a new datatype collated_text, it
could have a btree opfamily and no hash opfamily.  It's trickier with
only one datatype, but possibly we could come up with a way for an
opfamily to be consulted about whether it is available for a given
choice of collation.  I'm not exactly sure what is possible or
desirable, but I would not be too surprised to hear complaints about
the observed behavior different from the "pure" ICU behavior because
of the tiebreak, and at least some users might even find it worth
giving up hashing in order to get the exact sort order they need.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
@ 2017-06-09 16:18                     ` Peter Geoghegan <[email protected]>
  2017-06-09 17:45                       ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Peter Geoghegan @ 2017-06-09 16:18 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 9, 2017 at 9:17 AM, Robert Haas <[email protected]> wrote:
> I'm not exactly sure what is possible or
> desirable, but I would not be too surprised to hear complaints about
> the observed behavior different from the "pure" ICU behavior because
> of the tiebreak, and at least some users might even find it worth
> giving up hashing in order to get the exact sort order they need.

But they are getting the sort order they need. They just don't get the
equality semantics they expect.


-- 
Peter Geoghegan


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 16:18                     ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
@ 2017-06-09 17:45                       ` Robert Haas <[email protected]>
  2017-06-09 18:01                         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Robert Haas @ 2017-06-09 17:45 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 9, 2017 at 12:18 PM, Peter Geoghegan <[email protected]> wrote:
> On Fri, Jun 9, 2017 at 9:17 AM, Robert Haas <[email protected]> wrote:
>> I'm not exactly sure what is possible or
>> desirable, but I would not be too surprised to hear complaints about
>> the observed behavior different from the "pure" ICU behavior because
>> of the tiebreak, and at least some users might even find it worth
>> giving up hashing in order to get the exact sort order they need.
>
> But they are getting the sort order they need. They just don't get the
> equality semantics they expect.

You're right.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 16:18                     ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 17:45                       ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
@ 2017-06-09 18:01                         ` Peter Geoghegan <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Peter Geoghegan @ 2017-06-09 18:01 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 9, 2017 at 10:45 AM, Robert Haas <[email protected]> wrote:
>> But they are getting the sort order they need. They just don't get the
>> equality semantics they expect.
>
> You're right.

If we happened to ever guarantee the user a stable sort, then I'd be
wrong. We don't, though.


-- 
Peter Geoghegan


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
@ 2017-06-09 17:45                     ` Peter Eisentraut <[email protected]>
  2017-06-09 18:09                       ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  1 sibling, 1 reply; 33+ messages in thread

From: Peter Eisentraut @ 2017-06-09 17:45 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On 6/9/17 12:17, Robert Haas wrote:
> IOW, suppose there
> were a collation API call distill() which had the property that
> strcmp(distill(X), distill(Y)) == 0 iff X and Y are considered equal
> under that collation.  Then, you could define your hash function as
> hash_any(distill(X)).  Alternatively, if the collation library
> provided its own hashing function, that would be fine too, and
> probably faster.

Isn't that what strxfrm() is?

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


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: strcmp() tie-breaker for identical ICU-collated strings
  2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-01 21:24 ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-01 21:27   ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-01 21:48     ` Re: strcmp() tie-breaker for identical ICU-collated strings Thomas Munro <[email protected]>
  2017-06-02 17:34       ` Re: strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
  2017-06-02 18:22         ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Geoghegan <[email protected]>
  2017-06-09 14:31           ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 15:12             ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 15:29               ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
  2017-06-09 15:46                 ` Re: strcmp() tie-breaker for identical ICU-collated strings Tom Lane <[email protected]>
  2017-06-09 16:17                   ` Re: strcmp() tie-breaker for identical ICU-collated strings Robert Haas <[email protected]>
  2017-06-09 17:45                     ` Re: strcmp() tie-breaker for identical ICU-collated strings Peter Eisentraut <[email protected]>
@ 2017-06-09 18:09                       ` Robert Haas <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Robert Haas @ 2017-06-09 18:09 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; Amit Khandekar <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers

On Fri, Jun 9, 2017 at 1:45 PM, Peter Eisentraut
<[email protected]> wrote:
> On 6/9/17 12:17, Robert Haas wrote:
>> IOW, suppose there
>> were a collation API call distill() which had the property that
>> strcmp(distill(X), distill(Y)) == 0 iff X and Y are considered equal
>> under that collation.  Then, you could define your hash function as
>> hash_any(distill(X)).  Alternatively, if the collation library
>> provided its own hashing function, that would be fine too, and
>> probably faster.
>
> Isn't that what strxfrm() is?

Yeah, just with bugs.  If ICU has a non-buggy equivalent, then we can
make this work.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v35 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Mon_Jun__8_17_32_04_2020_489)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v35-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v33 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Thu_Apr_30_13_22_05_2020_409)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v33-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v32 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Tue_Apr__7_16_38_17_2020_299)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v32-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v36 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.4


----Next_Part(Tue_Sep__8_17_55_57_2020_197)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v36-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v25 1/8] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Thu_Mar_19_20_30_04_2020_284)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v25-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v29 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Wed_Apr__1_15_15_11_2020_923)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v29-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v31 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Fri_Apr__3_17_31_17_2020_104)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v31-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v34 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Mon_Jun__1_18_00_01_2020_089)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v34-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v33 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Fri_May_15_17_30_36_2020_111)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v33-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* [PATCH v30 1/7] Use standard crash handler in archiver.
@ 2020-03-16 08:15 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-16 08:15 UTC (permalink / raw)

The commit 8e19a82640 changed SIGQUIT handler of almost all processes
not to run atexit callbacks for safety. Archiver process should behave
the same way for the same reason. Exit status changes 1 to 2 but that
doesn't make any behavioral change.
---
 src/backend/postmaster/pgarch.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 01ffd6513c..37be0e2bbb 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,6 @@ static pid_t pgarch_forkexec(void);
 #endif
 
 NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_exit(SIGNAL_ARGS);
 static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
@@ -229,7 +228,7 @@ PgArchiverMain(int argc, char *argv[])
 	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGINT, SIG_IGN);
 	pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
-	pqsignal(SIGQUIT, pgarch_exit);
+	pqsignal(SIGQUIT, SignalHandlerForCrashExit);
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
 	pqsignal(SIGUSR1, pgarch_waken);
@@ -246,14 +245,6 @@ PgArchiverMain(int argc, char *argv[])
 	exit(0);
 }
 
-/* SIGQUIT signal handler for archiver process */
-static void
-pgarch_exit(SIGNAL_ARGS)
-{
-	/* SIGQUIT means curl up and die ... */
-	exit(1);
-}
-
 /* SIGUSR1 signal handler for archiver process */
 static void
 pgarch_waken(SIGNAL_ARGS)
-- 
2.18.2


----Next_Part(Wed_Apr__1_17_37_23_2020_570)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v30-0002-sequential-scan-for-dshash.patch"



^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: --sync-method isn't documented to take an argument
@ 2023-10-04 13:15 Daniel Gustafsson <[email protected]>
  2023-10-04 13:22 ` Re: --sync-method isn't documented to take an argument Robert Haas <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Daniel Gustafsson @ 2023-10-04 13:15 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: pgsql-hackers; Nathan Bossart <[email protected]>

> On 4 Oct 2023, at 15:08, Robert Haas <[email protected]> wrote:

> This one should be something like this:
> 
> <term><option>--sync-method=<replaceable>method</replaceable></option></term>

Shouldn't it be <replaceable class="parameter">method</replaceable> ?

--
Daniel Gustafsson







^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: --sync-method isn't documented to take an argument
  2023-10-04 13:15 Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
@ 2023-10-04 13:22 ` Robert Haas <[email protected]>
  2023-10-04 13:34   ` Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Robert Haas @ 2023-10-04 13:22 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: pgsql-hackers; Nathan Bossart <[email protected]>

On Wed, Oct 4, 2023 at 9:15 AM Daniel Gustafsson <[email protected]> wrote:
> > On 4 Oct 2023, at 15:08, Robert Haas <[email protected]> wrote:
> > This one should be something like this:
> >
> > <term><option>--sync-method=<replaceable>method</replaceable></option></term>
>
> Shouldn't it be <replaceable class="parameter">method</replaceable> ?

Hmm, I think you're probably right. But look at this:

      <term><option>-S <replaceable>slotname</replaceable></option></term>
      <term><option>--slot=<replaceable
class="parameter">slotname</replaceable></option></term>

But then in the very same file:

      <term><option>-r <replaceable
class="parameter">rate</replaceable></option></term>
      <term><option>--max-rate=<replaceable
class="parameter">rate</replaceable></option></term>

It doesn't look to me like we're entirely consistent about this. I
also found this in vacuumlo.sgml, and there seem to be various other
examples:

<term><option>-U <replaceable>username</replaceable></option></term>

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: --sync-method isn't documented to take an argument
  2023-10-04 13:15 Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
  2023-10-04 13:22 ` Re: --sync-method isn't documented to take an argument Robert Haas <[email protected]>
@ 2023-10-04 13:34   ` Daniel Gustafsson <[email protected]>
  2023-10-04 14:35     ` Re: --sync-method isn't documented to take an argument Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 33+ messages in thread

From: Daniel Gustafsson @ 2023-10-04 13:34 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: pgsql-hackers; Nathan Bossart <[email protected]>

> On 4 Oct 2023, at 15:22, Robert Haas <[email protected]> wrote:
> 
> On Wed, Oct 4, 2023 at 9:15 AM Daniel Gustafsson <[email protected]> wrote:
>>> On 4 Oct 2023, at 15:08, Robert Haas <[email protected]> wrote:
>>> This one should be something like this:
>>> 
>>> <term><option>--sync-method=<replaceable>method</replaceable></option></term>
>> 
>> Shouldn't it be <replaceable class="parameter">method</replaceable> ?
> 
> Hmm, I think you're probably right. But look at this:
> 
>      <term><option>-S <replaceable>slotname</replaceable></option></term>
>      <term><option>--slot=<replaceable
> class="parameter">slotname</replaceable></option></term>
> 
> But then in the very same file:
> 
>      <term><option>-r <replaceable
> class="parameter">rate</replaceable></option></term>
>      <term><option>--max-rate=<replaceable
> class="parameter">rate</replaceable></option></term>

Hmm.. that's a bit unfortunate.

> It doesn't look to me like we're entirely consistent about this.

That (sadly) applies to a fair chunk of the docs.

I can take a stab at tidying this up during breaks at the conference.  It might
not be the most important bit of markup, but for anyone building the docs who
might want to use this it seems consistency will help.

--
Daniel Gustafsson







^ permalink  raw  reply  [nested|flat] 33+ messages in thread

* Re: --sync-method isn't documented to take an argument
  2023-10-04 13:15 Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
  2023-10-04 13:22 ` Re: --sync-method isn't documented to take an argument Robert Haas <[email protected]>
  2023-10-04 13:34   ` Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
@ 2023-10-04 14:35     ` Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 33+ messages in thread

From: Alvaro Herrera @ 2023-10-04 14:35 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers; Nathan Bossart <[email protected]>

On 2023-Oct-04, Daniel Gustafsson wrote:

> I can take a stab at tidying this up during breaks at the conference.  It might
> not be the most important bit of markup, but for anyone building the docs who
> might want to use this it seems consistency will help.

So for HTML, the result of the pg_basebackup lines are these two lines:

</p></dd><dt><span class="term"><code class="option">-S <em class="replaceable"><code>slotname</code></em></code><br /></span><span class="term"><code class="option">--slot=<em class="replaceable"><code>slotname</code></em></code></span></dt><dd><p>

and 

</p></dd><dt><span class="term"><code class="option">-r <em class="replaceable"><code>rate</code></em></code><br /></span><span class="term"><code class="option">--max-rate=<em class="replaceable"><code>rate</code></em></code></span></dt><dd><p>

So I'm not sure that specifying the class="parameter" bit does anything in
reality, or that changing lines to add or remove it will have any effect.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/






^ permalink  raw  reply  [nested|flat] 33+ messages in thread


end of thread, other threads:[~2023-10-04 14:35 UTC | newest]

Thread overview: 33+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-06-01 18:58 strcmp() tie-breaker for identical ICU-collated strings Amit Khandekar <[email protected]>
2017-06-01 21:24 ` Thomas Munro <[email protected]>
2017-06-01 21:27   ` Peter Geoghegan <[email protected]>
2017-06-01 21:48     ` Tom Lane <[email protected]>
2017-06-01 21:48     ` Thomas Munro <[email protected]>
2017-06-02 17:34       ` Amit Khandekar <[email protected]>
2017-06-02 18:22         ` Peter Geoghegan <[email protected]>
2017-06-09 06:27           ` Amit Khandekar <[email protected]>
2017-06-09 14:31           ` Robert Haas <[email protected]>
2017-06-09 15:05             ` Peter Eisentraut <[email protected]>
2017-06-09 15:12             ` Tom Lane <[email protected]>
2017-06-09 15:29               ` Peter Eisentraut <[email protected]>
2017-06-09 15:46                 ` Tom Lane <[email protected]>
2017-06-09 16:17                   ` Robert Haas <[email protected]>
2017-06-09 16:18                     ` Peter Geoghegan <[email protected]>
2017-06-09 17:45                       ` Robert Haas <[email protected]>
2017-06-09 18:01                         ` Peter Geoghegan <[email protected]>
2017-06-09 17:45                     ` Peter Eisentraut <[email protected]>
2017-06-09 18:09                       ` Robert Haas <[email protected]>
2020-03-16 08:15 [PATCH v35 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v33 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v32 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v36 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v25 1/8] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v29 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v31 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v34 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v33 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2020-03-16 08:15 [PATCH v30 1/7] Use standard crash handler in archiver. Kyotaro Horiguchi <[email protected]>
2023-10-04 13:15 Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
2023-10-04 13:22 ` Re: --sync-method isn't documented to take an argument Robert Haas <[email protected]>
2023-10-04 13:34   ` Re: --sync-method isn't documented to take an argument Daniel Gustafsson <[email protected]>
2023-10-04 14:35     ` Re: --sync-method isn't documented to take an argument Alvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox