Hi,
Thanks for the detailed explanation!
On Mon, Aug 21, 2023 at 10:34 PM Önder Kalacı wrote:
>> As described in the commit message, we assume that extensions use the
>> hook in a similar way to FDWs
> I'm not sure if it is fair to assume that extensions use any hook in any way.
I am not sure
Hi,
Thanks for the explanation.
As described in the commit message, we assume that extensions use the
> hook in a similar way to FDWs
I'm not sure if it is fair to assume that extensions use any hook in any
way.
So my question is: does the Citus extension use the hook like this?
> (Sorry, I do
Sorry, I hit the send button by mistake.
On Sun, Aug 20, 2023 at 4:34 AM Andres Freund wrote:
> On 2023-08-19 20:09:25 +0900, Etsuro Fujita wrote:
> > * The problem we had with the set_join_pathlist_hook hook is that in
> > such a typical use case, previously, if the replaced joins had any
> > ps
Hi,
On Sun, Aug 20, 2023 at 4:34 AM Andres Freund wrote:
>
> Hi,
>
> On 2023-08-19 20:09:25 +0900, Etsuro Fujita wrote:
> > Maybe my explanation was not enough, so let me explain:
> >
> > * I think you could use the set_join_pathlist_hook hook as you like at
> > your own responsibility, but typic
Hi,
On 2023-08-19 20:09:25 +0900, Etsuro Fujita wrote:
> Maybe my explanation was not enough, so let me explain:
>
> * I think you could use the set_join_pathlist_hook hook as you like at
> your own responsibility, but typical use cases of the hook that are
> designed to support in the core syste
Hi Onder,
On Wed, Aug 16, 2023 at 10:58 PM Önder Kalacı wrote:
>> Maybe we could do so by leaving to extensions the decision whether
>> they replace joins with pseudoconstant clauses, but I am not sure that
>> that is a good idea, because that would require the authors to modify
>> and recompile
Hi Etsuro,
Thanks for the response!
> Maybe we could do so by leaving to extensions the decision whether
> they replace joins with pseudoconstant clauses, but I am not sure that
> that is a good idea, because that would require the authors to modify
> and recompile their extensions to fix the is
Hi,
On Tue, Aug 15, 2023 at 11:02 PM Önder Kalacı wrote:
> The commit[1] seems to break some queries in Citus[2], which is an extension
> which relies on set_join_pathlist_hook.
>
> Although the comment says /*Finally, give extensions a chance to manipulate
> the path list.*/ we use it to extr
Hi Etsuro, all
The commit[1] seems to break some queries in Citus[2], which is an
extension which relies on set_join_pathlist_hook.
Although the comment says */*Finally, give extensions a chance to
manipulate the path list.*/ *we use it to extract lots of information
about the joins and do the p
On Tue, Aug 8, 2023 at 6:30 PM Richard Guo wrote:
> On Tue, Aug 8, 2023 at 4:40 PM Etsuro Fujita wrote:
>> I modified the code a bit further to use an if-test to avoid a useless
>> function call, and added/tweaked comments and docs further. Attached
>> is a new version of the patch. I am planni
On Tue, Aug 8, 2023 at 4:40 PM Etsuro Fujita
wrote:
> I modified the code a bit further to use an if-test to avoid a useless
> function call, and added/tweaked comments and docs further. Attached
> is a new version of the patch. I am planning to commit this, if there
> are no objections.
+1 t
Hi Richard,
On Mon, Jul 31, 2023 at 5:52 PM Richard Guo wrote:
> On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita wrote:
>> 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_
On Fri, Jul 28, 2023 at 4:56 PM Etsuro Fujita
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
Hi Richard,
On Mon, Jul 24, 2023 at 11:45 AM Richard Guo wrote:
> On Fri, Jul 21, 2023 at 8:51 PM Etsuro Fujita wrote:
>> * In this bit I changed the last argument to NIL, which would be
>> nitpicking, though.
>>
>> @@ -1038,7 +1038,7 @@ postgresGetForeignPaths(PlannerInfo *root,
>> add_path(b
On Fri, Jul 21, 2023 at 8:51 PM Etsuro Fujita
wrote:
> I think we should choose the latter, so I modified your patch as
> mentioned, after re-creating it on top of my patch. Attached is a new
> version (0002-Allow-join-pushdown-even-if-pseudoconstant-quals-v2.patch).
> I am attaching my patch as
Hi Richard,
On Sun, Jun 25, 2023 at 3:05 PM Richard Guo wrote:
> On Wed, Jun 14, 2023 at 2:49 PM Etsuro Fujita wrote:
>> On Mon, Jun 5, 2023 at 10:19 PM Etsuro Fujita
>> wrote:
>> > To avoid this issue, I am wondering if we should modify
>> > add_paths_to_joinrel() in back branches so that it
On Wed, Jun 14, 2023 at 2:49 PM Etsuro Fujita
wrote:
> On Mon, Jun 5, 2023 at 10:19 PM Etsuro Fujita
> wrote:
> > To avoid this issue, I am wondering if we should modify
> > add_paths_to_joinrel() in back branches so that it just disallows the
> > FDW to consider pushing down joins when the rest
Looks good to me. Tested on master and it works.
New patch used a bool flag to avoid calls for both FDW and custom hook's
call. And a slight change in comment of "has_pseudoconstant_clauses"
function.
Regards,
Nishant.
On Wed, Jun 14, 2023 at 12:19 PM Etsuro Fujita
wrote:
> On Mon, Jun 5, 2023
On Mon, Jun 5, 2023 at 10:19 PM Etsuro Fujita wrote:
> To avoid this issue, I am wondering if we should modify
> add_paths_to_joinrel() in back branches so that it just disallows the
> FDW to consider pushing down joins when the restrictlist has
> pseudoconstant clauses. Attached is a patch for t
Hi,
On Wed, Jun 7, 2023 at 7:28 PM Nishant Sharma
wrote:
> Etsuro's patch is also showing the correct output for "set
> enable_nestloop=off". Looks good to me for back branches due to backport
> issues.
>
> But below are a few observations for the same:-
> 1) I looked into the query plan for bo
Hi Richard,
On Tue, Jun 6, 2023 at 12:20 PM Richard Guo wrote:
> On Mon, Jun 5, 2023 at 9:19 PM Etsuro Fujita wrote:
>> To avoid this issue, I am wondering if we should modify
>> add_paths_to_joinrel() in back branches so that it just disallows the
>> FDW to consider pushing down joins when the
Hi,
Etsuro's patch is also showing the correct output for "set
enable_nestloop=off". Looks good to me for back branches due to backport
issues.
But below are a few observations for the same:-
1) I looked into the query plan for both "set enable_nestloop" on & off
case and observe that they are t
On Mon, Jun 5, 2023 at 9:19 PM Etsuro Fujita
wrote:
> If the patch is intended for HEAD only, I also think it goes in the
> right direction. But if it is intended for back branches as well, I
> do not think so, because it would cause ABI breakage due to changes
> made to the ForeignPath struct a
Hi,
On Fri, Jun 2, 2023 at 9:31 PM Nishant Sharma
wrote:
> I also agree that Richard's patch is better. As it fixes the issue at the
> backend and does not treat pseudoconstant as local condition.
>
> I have tested Richard's patch and observe that it is resolving the problem.
> Patch looks good
On Fri, Jun 2, 2023 at 8:31 PM Nishant Sharma <
nishant.sha...@enterprisedb.com> wrote:
> *I only had a minor comment on below change:-*
>
>
>
>
>
> *- gating_clauses = get_gating_quals(root, scan_clauses);+ if
> (best_path->pathtype == T_ForeignScan && IS_JOIN_REL(rel))+
> gating_clauses = ge
On Fri, Jun 2, 2023 at 11:30 AM Suraj Kharage <
suraj.khar...@enterprisedb.com> wrote:
> +1 for fixing this in the backend code rather than FDW code.
>
> Thanks, Richard, for working on this. The patch looks good to me at
> a glance.
>
Thank you Suraj for the review!
Thanks
Richard
I also agree that Richard's patch is better. As it fixes the issue at the
backend and does not treat pseudoconstant as local condition.
I have tested Richard's patch and observe that it is resolving the problem.
Patch looks good to me as well.
*I only had a minor comment on below change:-*
*
+1 for fixing this in the backend code rather than FDW code.
Thanks, Richard, for working on this. The patch looks good to me at
a glance.
On Tue, Apr 25, 2023 at 3:36 PM Richard Guo wrote:
>
> On Fri, Apr 14, 2023 at 8:51 PM Etsuro Fujita
> wrote:
>
>> I think that the root cause for this iss
On Fri, Apr 14, 2023 at 8:51 PM Etsuro Fujita
wrote:
> I think that the root cause for this issue would be in the
> create_scan_plan handling of pseudoconstant quals when creating a
> foreign-join (or custom-join) plan.
Yes exactly. In create_scan_plan, we are supposed to extract all the
pseud
On Mon, Apr 24, 2023 at 3:31 PM Nishant Sharma
wrote:
> Any updates? -- did you get a chance to look into this?
Sorry, I have not looked into this yet, because I have been busy with
some other work recently. I plan to do so early next week.
Best regards,
Etsuro Fujita
Hi Etsuro Fujita,
Any updates? -- did you get a chance to look into this?
Regards,
Nishant.
On Mon, Apr 17, 2023 at 11:00 AM Nishant Sharma <
nishant.sha...@enterprisedb.com> wrote:
> Thanks Etsuro for your response!
>
> One small typo correction in my answer to "What is the technical issue?"
Thanks Etsuro for your response!
One small typo correction in my answer to "What is the technical issue?"
"it is *not* considered a pseudo constant" --> "it is considered a pseudo
constant"
Regards,
Nishant.
On Fri, Apr 14, 2023 at 6:21 PM Etsuro Fujita
wrote:
> Hi Nishant,
>
> On Fri, Apr 14
Hi Nishant,
On Fri, Apr 14, 2023 at 8:39 PM Nishant Sharma
wrote:
> I debugged this issue and was able to find a fix for the same. Kindly please
> refer to the attached fix. With the fix I am able to resolve the issue.
Thanks for the report and patch!
> What is the technical issue?
> The probl
Hi,
We have observed that running the same self JOIN query on postgres FDW
setup is returning different results with set enable_nestloop off & on. I
am at today's latest commit:- 928e05ddfd4031c67e101c5e74dbb5c8ec4f9e23
I created a local FDW setup. And ran this experiment on the same. Kindly
ref
34 matches
Mail list logo
