Re: Oversight in reparameterize_path_by_child leading to executor crash

Поиск
Список
Период
Сортировка
От Richard Guo
Тема Re: Oversight in reparameterize_path_by_child leading to executor crash
Дата
Msg-id CAMbWs48QmbYJ26_aULGnX4APfiynFmO8Jx49PAJNMUE6+1v_kw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Oversight in reparameterize_path_by_child leading to executor crash  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: Oversight in reparameterize_path_by_child leading to executor crash  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Thanks for the review!

On Sat, Jan 6, 2024 at 2:36 AM Robert Haas <robertmhaas@gmail.com> wrote:
Richard, I think it could be useful to put a better commit message
into the patch file, describing both what problem is being fixed and
what the design of the fix is. I gather that the problem is that we
crash if the query contains a partioningwise join and also $SOMETHING,
and the solution is to move reparameterization to happen at
createplan() time but with a precheck that runs during path
generation. Presumably, that means this is more than a minimal bug
fix, because the bug could be fixed without splitting
can-it-be-reparameterized to reparameterize-it in this way. Probably
that's a performance optimization, so maybe it's worth clarifying
whether that's just an independently good idea or whether it's a part
of making the bug fix not regress performance.

Thanks for the suggestion.  Attached is an updated patch which is added
with a commit message that tries to explain the problem and the fix.

I think the macro names in path_is_reparameterizable_by_child could be
better chosen. CHILD_PATH_IS_REPARAMETERIZABLE doesn't convey that the
macro will return from the calling function if not -- it looks like it
just returns a Boolean. Maybe REJECT_IF_PATH_NOT_REPARAMETERIZABLE and
REJECT_IF_PATH_LIST_NOT_REPARAMETERIZABLE or some such.

Agreed.
 
Another question here is whether we really want to back-patch all of
this or whether it might be better to, as Tom proposed previously,
back-patch a more minimal fix and leave the more invasive stuff for
master.

Fair point.  I think we can back-patch a more minimal fix, as Tom
proposed in [1], which disallows the reparameterization if the path
contains sampling info that references the outer rel.  But I think we
need also to disallow the reparameterization of a path if it contains
restriction clauses that reference the outer rel, as such paths have
been found to cause incorrect results, or planning errors as in [2].

[1] https://www.postgresql.org/message-id/3163033.1692719009%40sss.pgh.pa.us
[2] https://www.postgresql.org/message-id/CAMbWs4-CSR4VnZCDep3ReSoHGTA7E%2B3tnjF_LmHcX7yiGrkVfQ%40mail.gmail.com

Thanks
Richard
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: "Wilma Wantren"
Дата:
Сообщение: Changing a schema's name with function1 calling function2
Следующее
От: John Naylor
Дата:
Сообщение: Re: add AVX2 support to simd.h