Обсуждение: Crash in add_paths_to_append_rel

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

Crash in add_paths_to_append_rel

От
Richard Guo
Дата:
I came across a crash in add_paths_to_append_rel() with the query below.

create table t (a int);

create table inh (b int);
create table inh_child() inherits(inh);

explain (costs off)
select * from t left join lateral (select t.a from inh) on true limit 1;
server closed the connection unexpectedly

It was introduced by commit a8a968a8 where we referenced
cheapest_startup_path->param_info without first checking that we have a
valid cheapest_startup_path.  We do have checked that the pathlist is
not empty, but that does not imply that the cheapest_startup_path is not
NULL.  Normally only unparameterized paths are considered candidates for
cheapest_startup_path, because startup cost does not have too much value
for parameterized paths since they are on the inside of a nestloop.  So
if there are no unparameterized paths, the cheapest_startup_path will be
NULL.  That is the case in the repro query above.  Because of the
lateral reference within PHV, the child rels of 'inh' do not have
unparameterized paths, so their cheapest_startup_path is NULL.

I think we should explicitly check that cheapest_startup_path is not
NULL here.  Doing that seems to make the check of pathlist not being
empty unnecessary.  Besides, I think we do not need to check that
cheapest_startup_path->param_info is NULL, since cheapest_startup_path
must be unparameterized path.  Maybe we can use an Assert instead.

Attached is my proposed fix.

Thanks
Richard
Вложения

Re: Crash in add_paths_to_append_rel

От
David Rowley
Дата:
On Mon, 9 Oct 2023 at 22:49, Richard Guo <guofenglinux@gmail.com> wrote:
> I came across a crash in add_paths_to_append_rel() with the query below.

> It was introduced by commit a8a968a8 where we referenced
> cheapest_startup_path->param_info without first checking that we have a
> valid cheapest_startup_path.

Thank you for the report.

Since you've managed to attribute this to a specific commit, for the
future, I think instead of opening a new thread, it would be more
useful to communicate the issue on the thread that's linked in the
commit message.

I will look at this tomorrow.

David



Re: Crash in add_paths_to_append_rel

От
David Rowley
Дата:
On Mon, 9 Oct 2023 at 22:49, Richard Guo <guofenglinux@gmail.com> wrote:
> I came across a crash in add_paths_to_append_rel() with the query below.

> It was introduced by commit a8a968a8 where we referenced
> cheapest_startup_path->param_info without first checking that we have a
> valid cheapest_startup_path.

I pushed this with just minor adjustments to the comments you wrote. I
didn't feel the need to reiterate on the set_cheapest() comments.

David



Re: Crash in add_paths_to_append_rel

От
Andy Fan
Дата:


On Tue, Oct 10, 2023 at 11:52 AM David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 9 Oct 2023 at 22:49, Richard Guo <guofenglinux@gmail.com> wrote:
> I came across a crash in add_paths_to_append_rel() with the query below.

> It was introduced by commit a8a968a8 where we referenced
> cheapest_startup_path->param_info without first checking that we have a
> valid cheapest_startup_path.

I pushed this with just minor adjustments to the comments you wrote. I
didn't feel the need to reiterate on the set_cheapest() comments.

 
 Thanks Richard for the report and also Thanks you David for the push!

--
Best Regards
Andy Fan

Re: Crash in add_paths_to_append_rel

От
Richard Guo
Дата:

On Mon, Oct 9, 2023 at 7:35 PM David Rowley <dgrowleyml@gmail.com> wrote:
Since you've managed to attribute this to a specific commit, for the
future, I think instead of opening a new thread, it would be more
useful to communicate the issue on the thread that's linked in the
commit message.

Ah, yes, I should have done it that way, sorry.

Thanks
Richard

Re: Crash in add_paths_to_append_rel

От
Richard Guo
Дата:

On Tue, Oct 10, 2023 at 11:52 AM David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 9 Oct 2023 at 22:49, Richard Guo <guofenglinux@gmail.com> wrote:
> I came across a crash in add_paths_to_append_rel() with the query below.

> It was introduced by commit a8a968a8 where we referenced
> cheapest_startup_path->param_info without first checking that we have a
> valid cheapest_startup_path.

I pushed this with just minor adjustments to the comments you wrote. I
didn't feel the need to reiterate on the set_cheapest() comments.

Thanks for pushing!

Thanks
Richard