Re: Oversight in reparameterize_path_by_child leading to executor crash
От | Alena Rybakina |
---|---|
Тема | Re: Oversight in reparameterize_path_by_child leading to executor crash |
Дата | |
Msg-id | e41e3407-6e43-4ba3-8ad2-ead10ff442e9@yandex.ru обсуждение исходный текст |
Ответ на | Re: Oversight in reparameterize_path_by_child leading to executor crash (Richard Guo <guofenglinux@gmail.com>) |
Ответы |
Re: Oversight in reparameterize_path_by_child leading to executor crash
|
Список | pgsql-hackers |
Hi!
Thank you for your work on the subject.
During review your patch I didn't understand why are you checking that the variable is path and not new_path of type T_SamplePath (I highlighted it)?
Path *
reparameterize_path_by_child(PlannerInfo *root, Path *path,
RelOptInfo *child_rel)
...
switch (nodeTag(path))
{
case T_Path:
new_path = path;
ADJUST_CHILD_ATTRS(new_path->parent->baserestrictinfo);
if (path->pathtype == T_SampleScan)
{
Is it a typo and should be new_path?
Besides, it may not be important, but reviewing your code all the time stumbled on the statement of the comments while reading it (I highlighted it too). This:
* path_is_reparameterizable_by_child
* Given a path parameterized by the parent of the given child relation,
* see if it can be translated to be parameterized by the child relation.
*
* This must return true if and only if reparameterize_path_by_child()
* would succeed on this path.
Maybe is it better to rewrite it simplier:
* This must return true only if reparameterize_path_by_child()
* would succeed on this path.
And can we add assert in reparameterize_pathlist_by_child function that pathlist is not a NIL, because according to the comment it needs to be added there:Returns NIL to indicate failure, so pathlist had better not be NIL.
-- Regards, Alena Rybakina
В списке pgsql-hackers по дате отправления: