Crashing on insert to GIN index

Started by Jack Orensteinover 5 years ago3 messagesgeneral
Jump to latest
#1Jack Orenstein
jao@geophile.com

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jack Orenstein (#1)
Re: Crashing on insert to GIN index

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

#3Jack Orenstein
jao@geophile.com
In reply to: Tom Lane (#2)
Re: Crashing on insert to GIN index

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:

Show quoted text

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