Horton

Thread

Commits

  1. Add missing check_collation_set call to bpcharne().

  1. Unexpected "cache lookup failed for collation 0" failure

    Manuel Rigger <rigger.manuel@gmail.com> — 2019-11-13T16:01:56Z

    Hi everyone,

Is the error "cache lookup failed for collation 0" in this context expected?

CREATE TABLE t0(c0 CHAR(2) COLLATE "C", c1 CHAR(2) COLLATE "POSIX");
INSERT INTO t0 VALUES('', '');
SELECT * FROM t0 WHERE t0.c1 NOT IN (t0.c0); -- unexpected: cache
lookup failed for collation 0

This looks like an internal error message to me. I would expect an
"ERROR:  could not determine which collation to use for string
comparison", which is shown in similar situations (e.g., when
replacing the "NOT IN" by "IN"). This appears like a minor issue to me
though.

Best,
Manuel

  2. Re: Unexpected "cache lookup failed for collation 0" failure

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-13T16:50:52Z

    Manuel Rigger <rigger.manuel@gmail.com> writes:
> Is the error "cache lookup failed for collation 0" in this context expected?

> CREATE TABLE t0(c0 CHAR(2) COLLATE "C", c1 CHAR(2) COLLATE "POSIX");
> INSERT INTO t0 VALUES('', '');
> SELECT * FROM t0 WHERE t0.c1 NOT IN (t0.c0); -- unexpected: cache
> lookup failed for collation 0

No, that's never expected (except maybe in concurrent-drop scenarios).

> This looks like an internal error message to me.

I get an assertion failure :-(, although the assertion is just
complaining about the same thing, ie no-collation-assigned.
Used to work before v12, too.  Looking...

			regards, tom lane

  3. Re: Unexpected "cache lookup failed for collation 0" failure

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-13T20:56:11Z

    I wrote:
> Manuel Rigger <rigger.manuel@gmail.com> writes:
>> Is the error "cache lookup failed for collation 0" in this context expected?

>> CREATE TABLE t0(c0 CHAR(2) COLLATE "C", c1 CHAR(2) COLLATE "POSIX");
>> INSERT INTO t0 VALUES('', '');
>> SELECT * FROM t0 WHERE t0.c1 NOT IN (t0.c0); -- unexpected: cache
>> lookup failed for collation 0

> I get an assertion failure :-(, although the assertion is just
> complaining about the same thing, ie no-collation-assigned.
> Used to work before v12, too.  Looking...

Looks like a simple oversight --- when bpcharne() was made
collation-sensitive, it should have grown a check_collation_set()
call, but somehow that got left out.  Fixed.

			regards, tom lane

  4. Re: Unexpected "cache lookup failed for collation 0" failure

    Michael Paquier <michael@paquier.xyz> — 2019-11-14T04:03:42Z

    On Wed, Nov 13, 2019 at 03:56:11PM -0500, Tom Lane wrote:
> Looks like a simple oversight --- when bpcharne() was made
> collation-sensitive, it should have grown a check_collation_set()
> call, but somehow that got left out.  Fixed.

Wouldn't it be better to add a test case for that?
--
Michael

  5. Re: Unexpected "cache lookup failed for collation 0" failure

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-14T04:09:12Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Nov 13, 2019 at 03:56:11PM -0500, Tom Lane wrote:
>> Looks like a simple oversight --- when bpcharne() was made
>> collation-sensitive, it should have grown a check_collation_set()
>> call, but somehow that got left out.  Fixed.

> Wouldn't it be better to add a test case for that?

Didn't see the point particularly; we're not any more likely to
break this function than any other collation-dependent function.

The real question IMO is whether Peter missed any *other* places.
I dug through varlena.c and varchar.c and confirmed that every
call of PG_GET_COLLATION leads to a collation-is-not-zero test
(after this fix), but I didn't try to search the whole backend.

			regards, tom lane