Обсуждение: Re: backend crash on DELETE, reproducible locally
=?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes:
>>> Hm, what are the data types of those columns?
> scheduletemplate_id bigint NOT NULL,
> period_day smallint NOT NULL,
> timerange timerange NOT NULL,
OK, so here's a minimal reproducer:
drop table schedulecard;
create table schedulecard (
scheduletemplate_id bigint NOT NULL,
period_day smallint NOT NULL
);
CREATE INDEX schedulecard_overlap_idx
ON schedulecard USING gist
(scheduletemplate_id, (period_day::integer % 7));
insert into schedulecard values(12, 1);
update schedulecard set period_day = period_day + 7;
Interestingly, it doesn't crash if I change the index type to btree,
which I was not expecting because the crashing code seems pretty
independent of the index type.
Haven't traced further than that yet.
regards, tom lane
I wrote:
> Interestingly, it doesn't crash if I change the index type to btree,
> which I was not expecting because the crashing code seems pretty
> independent of the index type.
Oh ... duh. The problem here is that ProjIndexIsUnchanged thinks that
the type of the index column is identical to the type of the source
datum for it, which is not true for any opclass making use of the
opckeytype property.
Ondřej, as a short-term workaround you could prevent the crash
by setting that index's recheck_on_update property to false.
regards, tom lane
I wrote:
> Interestingly, it doesn't crash if I change the index type to btree,
> which I was not expecting because the crashing code seems pretty
> independent of the index type.
Oh ... duh. The problem here is that ProjIndexIsUnchanged thinks that
the type of the index column is identical to the type of the source
datum for it, which is not true for any opclass making use of the
opckeytype property.
Ondřej, as a short-term workaround you could prevent the crash
by setting that index's recheck_on_update property to false.
regards, tom lane
> Ondřej, as a short-term workaround you could prevent the crash
> by setting that index's recheck_on_update property to false.
Thanks for the tip. I am unsuccessful using it, though:
# ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update =
FALSE);
ERROR: unrecognized parameter "recheck_on_update"
Creating a new index is wrong, too:
# CREATE INDEX schedulecard_overlap_idx2
ON public.schedulecard USING gist
(scheduletemplate_id, (period_day::integer % 7), timerange)
WITH (recheck_on_update = FALSE);
ERROR: unrecognized parameter "recheck_on_update"
It only succeeds if not USING gist:
# CREATE INDEX schedulecard_overlap_idx2
ON public.schedulecard
(scheduletemplate_id, (period_day::integer % 7), timerange)
WITH (recheck_on_update = FALSE);
CREATE INDEX
Is there any other workaround for a gist index, please?
Maybe we will just drop the index until the bug gets fixed - better slow
queries than crashing servers...
Thanks,
Ondřej Bouda
> Ondřej, as a short-term workaround you could prevent the crash
> by setting that index's recheck_on_update property to false.
Thanks for the tip. I am unsuccessful using it, though:
# ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update =
FALSE);
ERROR: unrecognized parameter "recheck_on_update"
Creating a new index is wrong, too:
# CREATE INDEX schedulecard_overlap_idx2
ON public.schedulecard USING gist
(scheduletemplate_id, (period_day::integer % 7), timerange)
WITH (recheck_on_update = FALSE);
ERROR: unrecognized parameter "recheck_on_update"
It only succeeds if not USING gist:
# CREATE INDEX schedulecard_overlap_idx2
ON public.schedulecard
(scheduletemplate_id, (period_day::integer % 7), timerange)
WITH (recheck_on_update = FALSE);
CREATE INDEX
Is there any other workaround for a gist index, please?
Maybe we will just drop the index until the bug gets fixed - better slow
queries than crashing servers...
Thanks,
Ondřej Bouda
=?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes:
>> Ondřej, as a short-term workaround you could prevent the crash
>> by setting that index's recheck_on_update property to false.
> Thanks for the tip. I am unsuccessful using it, though:
> # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update =
> FALSE);
> ERROR: unrecognized parameter "recheck_on_update"
Oh, for crying out loud. That's yet a different bug.
I'm not sure that it's the fault of the recheck_on_update
feature proper though; it might be a pre-existing bug in
the reloptions code. Looks like somebody forgot to list
RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the
fault of commit c203d6cf8 or was it busted before?
regards, tom lane
=?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes:
>> Ondřej, as a short-term workaround you could prevent the crash
>> by setting that index's recheck_on_update property to false.
> Thanks for the tip. I am unsuccessful using it, though:
> # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update =
> FALSE);
> ERROR: unrecognized parameter "recheck_on_update"
Oh, for crying out loud. That's yet a different bug.
I'm not sure that it's the fault of the recheck_on_update
feature proper though; it might be a pre-existing bug in
the reloptions code. Looks like somebody forgot to list
RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the
fault of commit c203d6cf8 or was it busted before?
regards, tom lane
On 2018-Nov-06, Tom Lane wrote: > =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: > >> Ondřej, as a short-term workaround you could prevent the crash > >> by setting that index's recheck_on_update property to false. > > > Thanks for the tip. I am unsuccessful using it, though: > > # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = > > FALSE); > > ERROR: unrecognized parameter "recheck_on_update" > > Oh, for crying out loud. That's yet a different bug. > I'm not sure that it's the fault of the recheck_on_update > feature proper though; it might be a pre-existing bug in > the reloptions code. Looks like somebody forgot to list > RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > fault of commit c203d6cf8 or was it busted before? RELOPT_KIND_INDEX was invented by that commit, looks like :-( -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Nov-06, Tom Lane wrote: > =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: > >> Ondřej, as a short-term workaround you could prevent the crash > >> by setting that index's recheck_on_update property to false. > > > Thanks for the tip. I am unsuccessful using it, though: > > # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = > > FALSE); > > ERROR: unrecognized parameter "recheck_on_update" > > Oh, for crying out loud. That's yet a different bug. > I'm not sure that it's the fault of the recheck_on_update > feature proper though; it might be a pre-existing bug in > the reloptions code. Looks like somebody forgot to list > RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > fault of commit c203d6cf8 or was it busted before? RELOPT_KIND_INDEX was invented by that commit, looks like :-( -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-11-06 16:47:20 -0500, Tom Lane wrote: > =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: > >> Ondřej, as a short-term workaround you could prevent the crash > >> by setting that index's recheck_on_update property to false. > > > Thanks for the tip. I am unsuccessful using it, though: > > # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = > > FALSE); > > ERROR: unrecognized parameter "recheck_on_update" > > Oh, for crying out loud. That's yet a different bug. > I'm not sure that it's the fault of the recheck_on_update > feature proper though; it might be a pre-existing bug in > the reloptions code. Looks like somebody forgot to list > RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > fault of commit c203d6cf8 or was it busted before? Looks new: + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, there aren't any other "for all indexes" type options, so the whole category didn't exist before. It also strikes me as a really bad idea, even if RELOPT_KIND_GIST wouldn't have been omitted: It breaks index am extensibility. Greetings, Andres Freund
On 2018-11-06 16:47:20 -0500, Tom Lane wrote: > =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: > >> Ondřej, as a short-term workaround you could prevent the crash > >> by setting that index's recheck_on_update property to false. > > > Thanks for the tip. I am unsuccessful using it, though: > > # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = > > FALSE); > > ERROR: unrecognized parameter "recheck_on_update" > > Oh, for crying out loud. That's yet a different bug. > I'm not sure that it's the fault of the recheck_on_update > feature proper though; it might be a pre-existing bug in > the reloptions code. Looks like somebody forgot to list > RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > fault of commit c203d6cf8 or was it busted before? Looks new: + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, there aren't any other "for all indexes" type options, so the whole category didn't exist before. It also strikes me as a really bad idea, even if RELOPT_KIND_GIST wouldn't have been omitted: It breaks index am extensibility. Greetings, Andres Freund
On 11/6/18 10:54 PM, Andres Freund wrote: > On 2018-11-06 16:47:20 -0500, Tom Lane wrote: >> =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: >>>> Ondřej, as a short-term workaround you could prevent the crash >>>> by setting that index's recheck_on_update property to false. >> >>> Thanks for the tip. I am unsuccessful using it, though: >>> # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = >>> FALSE); >>> ERROR: unrecognized parameter "recheck_on_update" >> >> Oh, for crying out loud. That's yet a different bug. >> I'm not sure that it's the fault of the recheck_on_update >> feature proper though; it might be a pre-existing bug in >> the reloptions code. Looks like somebody forgot to list >> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the >> fault of commit c203d6cf8 or was it busted before? > > Looks new: > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > there aren't any other "for all indexes" type options, so the whole > category didn't exist before. > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > wouldn't have been omitted: It breaks index am extensibility. > Does it? The RELOPT_KIND_* stuff is hard-coded in reloptions.h anyway, so I'm not sure how this particular thing makes it less extensible? That being said, we also have RELOPT_KIND_BRIN, and that seems to be missing from RELOPT_KIND_INDEX too (and AFAICS the optimization works for all index types). regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 11/6/18 10:54 PM, Andres Freund wrote: > On 2018-11-06 16:47:20 -0500, Tom Lane wrote: >> =?UTF-8?Q?Ond=c5=99ej_Bouda?= <obouda@email.cz> writes: >>>> Ondřej, as a short-term workaround you could prevent the crash >>>> by setting that index's recheck_on_update property to false. >> >>> Thanks for the tip. I am unsuccessful using it, though: >>> # ALTER INDEX public.schedulecard_overlap_idx SET (recheck_on_update = >>> FALSE); >>> ERROR: unrecognized parameter "recheck_on_update" >> >> Oh, for crying out loud. That's yet a different bug. >> I'm not sure that it's the fault of the recheck_on_update >> feature proper though; it might be a pre-existing bug in >> the reloptions code. Looks like somebody forgot to list >> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the >> fault of commit c203d6cf8 or was it busted before? > > Looks new: > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > there aren't any other "for all indexes" type options, so the whole > category didn't exist before. > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > wouldn't have been omitted: It breaks index am extensibility. > Does it? The RELOPT_KIND_* stuff is hard-coded in reloptions.h anyway, so I'm not sure how this particular thing makes it less extensible? That being said, we also have RELOPT_KIND_BRIN, and that seems to be missing from RELOPT_KIND_INDEX too (and AFAICS the optimization works for all index types). regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Andres Freund <andres@anarazel.de> writes:
> On 2018-11-06 16:47:20 -0500, Tom Lane wrote:
>> Looks like somebody forgot to list
>> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the
>> fault of commit c203d6cf8 or was it busted before?
> Looks new:
> + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST,
> there aren't any other "for all indexes" type options, so the whole
> category didn't exist before.
> It also strikes me as a really bad idea, even if RELOPT_KIND_GIST
> wouldn't have been omitted: It breaks index am extensibility.
Hm, well, that enum already knows about all index types, doesn't it?
regards, tom lane
Andres Freund <andres@anarazel.de> writes:
> On 2018-11-06 16:47:20 -0500, Tom Lane wrote:
>> Looks like somebody forgot to list
>> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the
>> fault of commit c203d6cf8 or was it busted before?
> Looks new:
> + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST,
> there aren't any other "for all indexes" type options, so the whole
> category didn't exist before.
> It also strikes me as a really bad idea, even if RELOPT_KIND_GIST
> wouldn't have been omitted: It breaks index am extensibility.
Hm, well, that enum already knows about all index types, doesn't it?
regards, tom lane
Hi, On 2018-11-06 23:11:29 +0100, Tomas Vondra wrote: > On 11/6/18 10:54 PM, Andres Freund wrote: > > Looks new: > > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > > > there aren't any other "for all indexes" type options, so the whole > > category didn't exist before. > > > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > > wouldn't have been omitted: It breaks index am extensibility. > > > > Does it? The RELOPT_KIND_* stuff is hard-coded in reloptions.h anyway, > so I'm not sure how this particular thing makes it less extensible? Well, you can create new index AMs in extensions these days, but given the relopt design above, the feature cannot be disabled for them. Yes, there's *currently* probably no great way to have reloptions across all potential index types, but that's not an excuse for adding something broken. Greetings, Andres Freund
Hi, On 2018-11-06 23:11:29 +0100, Tomas Vondra wrote: > On 11/6/18 10:54 PM, Andres Freund wrote: > > Looks new: > > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > > > there aren't any other "for all indexes" type options, so the whole > > category didn't exist before. > > > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > > wouldn't have been omitted: It breaks index am extensibility. > > > > Does it? The RELOPT_KIND_* stuff is hard-coded in reloptions.h anyway, > so I'm not sure how this particular thing makes it less extensible? Well, you can create new index AMs in extensions these days, but given the relopt design above, the feature cannot be disabled for them. Yes, there's *currently* probably no great way to have reloptions across all potential index types, but that's not an excuse for adding something broken. Greetings, Andres Freund
Hi, On 2018-11-06 17:11:40 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2018-11-06 16:47:20 -0500, Tom Lane wrote: > >> Looks like somebody forgot to list > >> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > >> fault of commit c203d6cf8 or was it busted before? > > > Looks new: > > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > there aren't any other "for all indexes" type options, so the whole > > category didn't exist before. > > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > > wouldn't have been omitted: It breaks index am extensibility. > > Hm, well, that enum already knows about all index types, doesn't it? Not quite sure what you mean. Before c203d6cf8 there weren't reloptions across index types. But after it, if one adds a new index AM via an extension, one can't set recheck_on_update = false for indexes using it, even though the feature affects such indexes. We support adding indexes AMs at runtime these days, including WAL logging (even though that's a bit voluminous). There's even a contrib index AM... The list of AMs is supposed to be extensible at runtime, cf add_reloption_kind(). Greetings, Andres Freund
Hi, On 2018-11-06 17:11:40 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2018-11-06 16:47:20 -0500, Tom Lane wrote: > >> Looks like somebody forgot to list > >> RELOPT_KIND_GIST in RELOPT_KIND_INDEX, but is that the > >> fault of commit c203d6cf8 or was it busted before? > > > Looks new: > > + RELOPT_KIND_INDEX = RELOPT_KIND_BTREE|RELOPT_KIND_HASH|RELOPT_KIND_GIN|RELOPT_KIND_SPGIST, > > there aren't any other "for all indexes" type options, so the whole > > category didn't exist before. > > > It also strikes me as a really bad idea, even if RELOPT_KIND_GIST > > wouldn't have been omitted: It breaks index am extensibility. > > Hm, well, that enum already knows about all index types, doesn't it? Not quite sure what you mean. Before c203d6cf8 there weren't reloptions across index types. But after it, if one adds a new index AM via an extension, one can't set recheck_on_update = false for indexes using it, even though the feature affects such indexes. We support adding indexes AMs at runtime these days, including WAL logging (even though that's a bit voluminous). There's even a contrib index AM... The list of AMs is supposed to be extensible at runtime, cf add_reloption_kind(). Greetings, Andres Freund