Re: postgres_fdw: wrong results with self join + enable_nestloop off
От | Richard Guo |
---|---|
Тема | Re: postgres_fdw: wrong results with self join + enable_nestloop off |
Дата | |
Msg-id | CAMbWs4-3iptx7oJDFF1ACSU-1SjcahHxfs62kve4N=KCnazVYQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: postgres_fdw: wrong results with self join + enable_nestloop off (Etsuro Fujita <etsuro.fujita@gmail.com>) |
Ответы |
Re: postgres_fdw: wrong results with self join + enable_nestloop off
|
Список | pgsql-hackers |
On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
Cool! I pushed the first patch after polishing it a little bit, so
here is a rebased version of the second patch, in which I modified the
ForeignPath and CustomPath cases in reparameterize_path_by_child() to
reflect the new members fdw_restrictinfo and custom_restrictinfo, for
safety, and tweaked a comment a bit.
Hmm, it seems that ForeignPath for a foreign join does not support
parameterized paths for now, as in postgresGetForeignJoinPaths() we have
this check:
/*
* This code does not work for joins with lateral references, since those
* must have parameterized paths, which we don't generate yet.
*/
if (!bms_is_empty(joinrel->lateral_relids))
return;
And in create_foreign_join_path() we just set the path.param_info to
NULL.
pathnode->path.param_info = NULL; /* XXX see above */
So I doubt that it's necessary to adjust fdw_restrictinfo in
reparameterize_path_by_child, because it seems to me that
fdw_restrictinfo must be empty there. Maybe we can add an Assert there
as below:
- ADJUST_CHILD_ATTRS(fpath->fdw_restrictinfo);
+
+ /*
+ * Parameterized foreign joins are not supported. So this
+ * ForeignPath cannot be a foreign join and fdw_restrictinfo
+ * must be empty.
+ */
+ Assert(fpath->fdw_restrictinfo == NIL);
That being said, it's also no harm to handle fdw_restrictinfo in
reparameterize_path_by_child as the patch does. So I'm OK we do that
for safety.
Thanks
Richard
parameterized paths for now, as in postgresGetForeignJoinPaths() we have
this check:
/*
* This code does not work for joins with lateral references, since those
* must have parameterized paths, which we don't generate yet.
*/
if (!bms_is_empty(joinrel->lateral_relids))
return;
And in create_foreign_join_path() we just set the path.param_info to
NULL.
pathnode->path.param_info = NULL; /* XXX see above */
So I doubt that it's necessary to adjust fdw_restrictinfo in
reparameterize_path_by_child, because it seems to me that
fdw_restrictinfo must be empty there. Maybe we can add an Assert there
as below:
- ADJUST_CHILD_ATTRS(fpath->fdw_restrictinfo);
+
+ /*
+ * Parameterized foreign joins are not supported. So this
+ * ForeignPath cannot be a foreign join and fdw_restrictinfo
+ * must be empty.
+ */
+ Assert(fpath->fdw_restrictinfo == NIL);
That being said, it's also no harm to handle fdw_restrictinfo in
reparameterize_path_by_child as the patch does. So I'm OK we do that
for safety.
Thanks
Richard
В списке pgsql-hackers по дате отправления: