Обсуждение: Crashing on insert to GIN index

Поиск
Список
Период
Сортировка

Crashing on insert to GIN index

От
Jack Orenstein
Дата:
I am defining a new type, FooBar, and trying to create a GIN index for it. Everything is working well without the index. FooBar values are getting into a table, and being retrieved and selected correctly. But I'm getting a crash when I add a GIN index on a column of type FooBar.

Here is the operator class:

create operator class foobar_ops
default for type foobar using gin
as
        operator 1 @@,
        function 1 foobar_cmp(bigint, bigint),
        function 2 foobar_item_to_keys(foobar, internal),
        function 3 foobar_query_to_keys(foobar, internal, int2, internal, internal),
        function 4 foobar_match(internal, int2, anyelement, int4, internal, internal),
        function 5 foobar_partial_match(foobar, foobar, int2, internal);

Here is the postgres function for extracting keys from FooBar values:

create function foobar_item_to_keys(foobar, internal) returns internal
as '$libdir/foobar'
language C immutable strict parallel safe;

And the implementation:

Datum foobar_item_to_keys(PG_FUNCTION_ARGS)
{
    FooBar* foobar = (FooBar*) DatumGetPointer(PG_GETARG_DATUM(0));
    int32* n_keys = (int32*) PG_GETARG_POINTER(1);
    int64_t* keys = (int64_t*) palloc(sizeof(int64_t));
    *n_keys = 1;
    keys[0] = foobar->key0;
    PG_RETURN_POINTER(keys);
}

(Eventually there will be multiple keys, so it really does need to be a GIN index.)

I have used ereport debugging to prove that the FooBar delivered into foobar_item_to_keys is correct, and that the PG_RETURN_POINTER statement is being reached.

I have been reading the Postgres docs, and comparing my code to the examples in contrib, and cannot see what I'm doing wrong. Can anyone see a problem in what I've described? Or point me in the right direction to debug this problem?

Thanks.

Jack Orenstein

Re: Crashing on insert to GIN index

От
Tom Lane
Дата:
Jack Orenstein <jao@geophile.com> writes:
> I am defining a new type, FooBar, and trying to create a GIN index for it.
> Everything is working well without the index. FooBar values are getting
> into a table, and being retrieved and selected correctly. But I'm getting a
> crash when I add a GIN index on a column of type FooBar.

> Here is the operator class:

> create operator class foobar_ops
> default for type foobar using gin
> as
>         operator 1 @@,
>         function 1 foobar_cmp(bigint, bigint),
>         function 2 foobar_item_to_keys(foobar, internal),
>         function 3 foobar_query_to_keys(foobar, internal, int2, internal,
> internal),
>         function 4 foobar_match(internal, int2, anyelement, int4, internal,
> internal),
>         function 5 foobar_partial_match(foobar, foobar, int2, internal);

Hmm, don't you want a "STORAGE bigint" clause in there?

> And the implementation:

>     int64_t* keys = (int64_t*) palloc(sizeof(int64_t));

As a general rule, ignoring the conventions about how to use Datums
is a good way to cause yourself pain.  It doesn't look like what
you've shown us so far is directly broken ... as long as you don't
try to run it on 32-bit hardware ... but bugs could easily be lurking
nearby.  More, the fact that this code looks nothing like standard
coding for the task is not making your life easier, because you
can't easily compare what you've done to other functions.  It'd be
much wiser to write this as

    Datum *keys = (Datum *) palloc(sizeof(Datum) * whatever);

and then use Int64GetDatum() to convert your integer key
values to Datums.  Yes, I'm well aware that that macro is
physically a no-op (... on 64-bit hardware ...) but you're
best advised to not rely on that, but think of Datum as a
physically distinct type.

            regards, tom lane

Re: Crashing on insert to GIN index

От
Jack Orenstein
Дата:
Thank you, the missing STORAGE clause was the problem.

As for the non-standard coding: I did start out with more correct coding, and it wandered off as I tried to figure out what was causing the crash.

Jack Orenstein

On Sun, Jan 3, 2021 at 7:57 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Jack Orenstein <jao@geophile.com> writes:
> I am defining a new type, FooBar, and trying to create a GIN index for it.
> Everything is working well without the index. FooBar values are getting
> into a table, and being retrieved and selected correctly. But I'm getting a
> crash when I add a GIN index on a column of type FooBar.

> Here is the operator class:

> create operator class foobar_ops
> default for type foobar using gin
> as
>         operator 1 @@,
>         function 1 foobar_cmp(bigint, bigint),
>         function 2 foobar_item_to_keys(foobar, internal),
>         function 3 foobar_query_to_keys(foobar, internal, int2, internal,
> internal),
>         function 4 foobar_match(internal, int2, anyelement, int4, internal,
> internal),
>         function 5 foobar_partial_match(foobar, foobar, int2, internal);

Hmm, don't you want a "STORAGE bigint" clause in there?

> And the implementation:

>     int64_t* keys = (int64_t*) palloc(sizeof(int64_t));

As a general rule, ignoring the conventions about how to use Datums
is a good way to cause yourself pain.  It doesn't look like what
you've shown us so far is directly broken ... as long as you don't
try to run it on 32-bit hardware ... but bugs could easily be lurking
nearby.  More, the fact that this code looks nothing like standard
coding for the task is not making your life easier, because you
can't easily compare what you've done to other functions.  It'd be
much wiser to write this as

        Datum *keys = (Datum *) palloc(sizeof(Datum) * whatever);

and then use Int64GetDatum() to convert your integer key
values to Datums.  Yes, I'm well aware that that macro is
physically a no-op (... on 64-bit hardware ...) but you're
best advised to not rely on that, but think of Datum as a
physically distinct type.

                        regards, tom lane