> On 08 Apr 2017, at 16:14, David Steele wrote:
>
> On 3/22/17 6:20 AM, Etsuro Fujita wrote:
>> On 2017/02/22 19:57, Rushabh Lathia wrote:
>>> Marked this as Ready for Committer.
>>
>> I noticed that this item in the CF app was incorrectly marked as
>> Committed. This
On 3/22/17 6:20 AM, Etsuro Fujita wrote:
> On 2017/02/22 19:57, Rushabh Lathia wrote:
>> Marked this as Ready for Committer.
>
> I noticed that this item in the CF app was incorrectly marked as
> Committed. This patch isn't committed, so I returned it to the previous
> status. I also rebased
On Wed, Mar 22, 2017 at 6:20 AM, Etsuro Fujita
wrote:
> On 2017/02/22 19:57, Rushabh Lathia wrote:
>> Marked this as Ready for Committer.
>
> I noticed that this item in the CF app was incorrectly marked as Committed.
> This patch isn't committed, so I returned it to
On 2017/02/22 19:57, Rushabh Lathia wrote:
Marked this as Ready for Committer.
I noticed that this item in the CF app was incorrectly marked as
Committed. This patch isn't committed, so I returned it to the previous
status. I also rebased the patch. Attached is a new version of the patch.
On 2017/03/17 2:35, Robert Haas wrote:
And ... I don't see anything to complain about, so, committed.
Thanks for committing, Robert! Thanks for reviewing, Ashutosh and David!
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes
On Thu, Mar 16, 2017 at 12:48 PM, Robert Haas wrote:
> On Thu, Mar 16, 2017 at 11:46 AM, David Steele wrote:
>> $ patch -p1 < ../other/postgres-fdw-subquery-support-v15.patch
>> patching file contrib/postgres_fdw/deparse.c
>> Hunk #11 succeeded at 1371
On Thu, Mar 16, 2017 at 11:46 AM, David Steele wrote:
> $ patch -p1 < ../other/postgres-fdw-subquery-support-v15.patch
> patching file contrib/postgres_fdw/deparse.c
> Hunk #11 succeeded at 1371 (offset -3 lines).
> Hunk #12 succeeded at 1419 (offset -3 lines).
> Hunk #13
On 1/30/17 6:30 AM, Etsuro Fujita wrote:
> On 2017/01/27 21:25, Etsuro Fujita wrote:
>> On 2017/01/27 20:04, Ashutosh Bapat wrote:
>>> I think we should pick up your patch on
>>> 27th December, update the comment per your mail on 5th Jan. I will
>>> review it once and list down the things left to
On Wed, Feb 22, 2017 at 12:15 PM, Etsuro Fujita wrote:
> On 2017/02/21 19:31, Rushabh Lathia wrote:
>
>> On Mon, Feb 20, 2017 at 1:41 PM, Etsuro Fujita
>> > wrote:
>>
>
> On 2017/02/13 18:24,
On 2017/02/21 19:31, Rushabh Lathia wrote:
On Mon, Feb 20, 2017 at 1:41 PM, Etsuro Fujita
> wrote:
On 2017/02/13 18:24, Rushabh Lathia wrote:
Here are few comments:
1)
@@ -211,6 +211,12 @@ typedef
On Mon, Feb 20, 2017 at 1:41 PM, Etsuro Fujita
wrote:
> On 2017/02/13 18:24, Rushabh Lathia wrote:
>
>> I started reviewing the patch again. Patch applied cleanly on latest
>> source
>> as well as regression pass through with the patch. I also performed
>> few manual
On 2017/02/13 18:24, Rushabh Lathia wrote:
I started reviewing the patch again. Patch applied cleanly on latest source
as well as regression pass through with the patch. I also performed
few manual testing and haven't found any regression. Patch look
much cleaner the earlier version, and don't
Sorry for delay in the review.
I started reviewing the patch again. Patch applied cleanly on latest source
as well as regression pass through with the patch. I also performed
few manual testing and haven't found any regression. Patch look
much cleaner the earlier version, and don't have any major
On Wed, Jan 25, 2017 at 7:20 PM, Etsuro Fujita
wrote:
> Attached is the new version of the patch. I also addressed other comments
> from you: moved rewriting the fdw_scan_tlist to postgres_fdw.c,
> added/revised comments, and added regression tests for the case where
On Mon, Jan 30, 2017 at 8:30 PM, Etsuro Fujita
wrote:
> Other changes:
> * I went back to make_outerrel_subquery and make_innerrel_subquery, which
> are flags to indicate whether to deparse the input relations as subqueries.
> is_subquery_rel would work well for
On 2017/01/30 21:05, Ashutosh Bapat wrote:
On Mon, Jan 30, 2017 at 5:00 PM, Etsuro Fujita
wrote:
On 2017/01/27 21:25, Etsuro Fujita wrote:
Sorry, I started thinking we went in the wrong direction. I added to
deparseSelectStmtForRel build_subquery_tlists, which
On Mon, Jan 30, 2017 at 5:00 PM, Etsuro Fujita
wrote:
> On 2017/01/27 21:25, Etsuro Fujita wrote:
>>
>> On 2017/01/27 20:04, Ashutosh Bapat wrote:
>>>
>>> I think we should pick up your patch on
>>> 27th December, update the comment per your mail on 5th Jan. I will
On 2017/01/27 21:25, Etsuro Fujita wrote:
On 2017/01/27 20:04, Ashutosh Bapat wrote:
I think we should pick up your patch on
27th December, update the comment per your mail on 5th Jan. I will
review it once and list down the things left to committer's judgement.
Sorry, I started thinking we
On 2017/01/27 20:04, Ashutosh Bapat wrote:
On Fri, Jan 13, 2017 at 12:30 PM, Etsuro Fujita
wrote:
A more clean way I'm thinking is: (1) in
postgresGetForeignJoinPaths(), create a tlist by build_tlist_to_deparse()
and save it in fpinfo->tlist before
On Fri, Jan 13, 2017 at 12:30 PM, Etsuro Fujita
wrote:
> On 2017/01/12 18:25, Ashutosh Bapat wrote:
>>
>> On Wed, Jan 11, 2017 at 5:45 PM, Etsuro Fujita
>> wrote:
>
>
> On 2017/01/05 21:11, Ashutosh Bapat wrote:
>>
>> IIUC,
On 2016/11/30 17:29, Etsuro Fujita wrote:
On 2016/11/23 20:28, Rushabh Lathia wrote:
I wrote:
How about inserting that before the
out param **retrieved_attrs, like this?
static void
deparseExplicitTargetList(List *tlist,
bool is_returning,
On 2017/01/12 18:25, Ashutosh Bapat wrote:
On Wed, Jan 11, 2017 at 5:45 PM, Etsuro Fujita
wrote:
On 2017/01/05 21:11, Ashutosh Bapat wrote:
IIUC, for a relation with use_remote_estimates we will deparse the
query twice and will build the targetlist twice.
On Wed, Jan 11, 2017 at 5:45 PM, Etsuro Fujita
wrote:
> On 2017/01/05 21:38, Ashutosh Bapat wrote:
>>
>> On Thu, Jan 5, 2017 at 5:51 PM, Etsuro Fujita
>> wrote:
>>>
>>> On 2017/01/05 21:11, Ashutosh Bapat wrote:
>
>
> On 2017/01/03
On 2017/01/05 21:38, Ashutosh Bapat wrote:
On Thu, Jan 5, 2017 at 5:51 PM, Etsuro Fujita
wrote:
On 2017/01/05 21:11, Ashutosh Bapat wrote:
On 2017/01/03 17:28, Ashutosh Bapat wrote:
Also, in this function, if fpinfo->tlist is already set, why do we want
to
On Thu, Jan 5, 2017 at 5:51 PM, Etsuro Fujita
wrote:
> On 2017/01/05 21:11, Ashutosh Bapat wrote:
>>
>> On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita
>> wrote:
>>>
>>> On 2017/01/03 17:28, Ashutosh Bapat wrote:
In
On 2017/01/05 21:11, Ashutosh Bapat wrote:
On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita
wrote:
On 2017/01/03 17:28, Ashutosh Bapat wrote:
In build_subquery_tlists(), why don't we handle base relations?
+ if (foreignrel->reloptkind != RELOPT_JOINREL)
+
On Thu, Jan 5, 2017 at 5:14 PM, Etsuro Fujita
wrote:
> On 2017/01/03 17:28, Ashutosh Bapat wrote:
>
> I wrote:
>>>
>>> I updated the patch a bit further: simplified the function name
>>> (s/build_subquery_rel_tlists/build_subquery_tlists/), and revised
>>> comments a
On 2017/01/03 17:28, Ashutosh Bapat wrote:
I wrote:
I updated the patch a bit further: simplified the function name
(s/build_subquery_rel_tlists/build_subquery_tlists/), and revised comments a
little bit. Attached is an updated version
(postgres-fdw-subquery-support-v14.patch).
Few comments
>
> I updated the patch a bit further: simplified the function name
> (s/build_subquery_rel_tlists/build_subquery_tlists/), and revised comments a
> little bit. Attached is an updated version
> (postgres-fdw-subquery-support-v14.patch). And I rebased another patch for
> PHVs against that patch,
On 2016/12/08 21:08, Etsuro Fujita wrote:
On 2016/12/07 20:23, Etsuro Fujita wrote:
My proposal here would be a bit different from what you proposed; right
before deparseSelectSql in deparseSelectStmtForRel, build the tlists for
relations present in the given jointree that will be deparsed as
On 2016/12/07 20:23, Etsuro Fujita wrote:
On 2016/12/07 15:39, Ashutosh Bapat wrote:
On 2016/11/22 18:28, Ashutosh Bapat wrote:
I guess, the reason why you are doing it this way, is SELECT clause for
the
outermost query gets deparsed before FROM clause. For later we call
On 2016/12/07 21:11, Ashutosh Bapat wrote:
On Wed, Dec 7, 2016 at 4:51 PM, Etsuro Fujita
wrote:
On 2016/12/05 20:20, Ashutosh Bapat wrote:
3. Why is foreignrel variable changed to rel?
-extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
-
On Wed, Dec 7, 2016 at 4:51 PM, Etsuro Fujita
wrote:
> On 2016/12/05 20:20, Ashutosh Bapat wrote:
>>
>> On Mon, Nov 28, 2016 at 3:52 PM, Etsuro Fujita
>> wrote:
>>>
>>> On 2016/11/24 21:45, Etsuro Fujita wrote:
* I removed
On 2016/12/07 15:39, Ashutosh Bapat wrote:
On 2016/11/22 18:28, Ashutosh Bapat wrote:
I guess, the reason why you are doing it this way, is SELECT clause for
the
outermost query gets deparsed before FROM clause. For later we call
deparseRangeTblRef(), which builds the tlist. So, while
On 2016/12/05 20:20, Ashutosh Bapat wrote:
On Mon, Nov 28, 2016 at 3:52 PM, Etsuro Fujita
wrote:
On 2016/11/24 21:45, Etsuro Fujita wrote:
* I removed make_outerrel_subquery/make_innerrel_subquery from fpinfo
and added a new member "is_subquery_rel" to fpinfo, as
>
>
> Ashutosh proposed this to do the comparison:
>
> On 2016/11/22 18:28, Ashutosh Bapat wrote:
>>
>> I guess, the reason why you are doing it this way, is SELECT clause for
>> the
>> outermost query gets deparsed before FROM clause. For later we call
>> deparseRangeTblRef(), which builds the
On Wed, Dec 7, 2016 at 1:57 AM, Robert Haas wrote:
> On Mon, Dec 5, 2016 at 6:20 AM, Ashutosh Bapat
> wrote:
>> 4. I am still not happy with this change
>> +/*
>> + * Since (1) the expressions in foreignrel's reltarget
On 2016/12/07 5:27, Robert Haas wrote:
On Mon, Dec 5, 2016 at 6:20 AM, Ashutosh Bapat
wrote:
4. I am still not happy with this change
+/*
+ * Since (1) the expressions in foreignrel's reltarget doesn't contain
+ * any PHVs and (2)
On Mon, Dec 5, 2016 at 6:20 AM, Ashutosh Bapat
wrote:
> 4. I am still not happy with this change
> +/*
> + * Since (1) the expressions in foreignrel's reltarget doesn't
> contain
> + * any PHVs and (2) foreignrel's local_conds is empty,
On Mon, Nov 28, 2016 at 3:52 PM, Etsuro Fujita
wrote:
> On 2016/11/24 21:45, Etsuro Fujita wrote:
>>
>> On 2016/11/22 18:28, Ashutosh Bapat wrote:
>>>
>>> The comments should explain why is the assertion true.
>>> +/* Shouldn't be NIL */
>>> +
On 2016/11/23 20:28, Rushabh Lathia wrote:
On Tue, Nov 22, 2016 at 6:24 PM, Etsuro Fujita
> wrote:
1)
-static void deparseExplicitTargetList(List *tlist, List
**retrieved_attrs,
+static void
On 2016/11/24 21:45, Etsuro Fujita wrote:
On 2016/11/22 18:28, Ashutosh Bapat wrote:
The comments should explain why is the assertion true.
+/* Shouldn't be NIL */
+Assert(tlist != NIL);
I noticed that I was wrong; in the Assertion the tlist can be empty. An
example for such
On 2016/11/22 18:28, Ashutosh Bapat wrote:
The comments should explain why is the assertion true.
+/* Shouldn't be NIL */
+Assert(tlist != NIL);
I noticed that I was wrong; in the Assertion the tlist can be empty. An
example for such a case is:
SELECT 1 FROM (SELECT c1 FROM
On 2016/11/24 18:20, Ashutosh Bapat wrote:
I wrote:
You missed the point; the foreignrel->reltarget->exprs doesn't contain
any
PHVs, so the tlist created by build_tlist_to_depase will be guaranteed to
be
one-to-one with the foreignrel->reltarget->exprs.
You wrote:
It's guaranteed now, but
>
>
build_tlist_to_depase() calls pull_var_nodes() before creating the
tlist, whereas the code that searches does not do that. Code-wise
those are not the same things.
>
>
>>> You missed the point; the foreignrel->reltarget->exprs doesn't contain
>>> any
>>> PHVs, so the tlist
On 2016/11/24 17:39, Ashutosh Bapat wrote:
On Thu, Nov 24, 2016 at 1:27 PM, Etsuro Fujita
wrote:
On 2016/11/24 16:46, Ashutosh Bapat wrote:
table will be misleading as subquery can represent a join and
corresponding alias would represent the join. Relation is
On Thu, Nov 24, 2016 at 1:27 PM, Etsuro Fujita
wrote:
> On 2016/11/24 16:46, Ashutosh Bapat wrote:
Sorry. I think the current version is better than previous one. The
term "subselect alias" is confusing in the previous version. In the
current
On 2016/11/24 16:46, Ashutosh Bapat wrote:
Sorry. I think the current version is better than previous one. The
term "subselect alias" is confusing in the previous version. In the
current version, "Get the relation and column alias for a given Var
node," we need to add word "identifiers" like
>
>> Sorry. I think the current version is better than previous one. The
>> term "subselect alias" is confusing in the previous version. In the
>> current version, "Get the relation and column alias for a given Var
>> node," we need to add word "identifiers" like "Get the relation and
>> column
On 2016/11/23 0:30, Ashutosh Bapat wrote:
You wrote:
The comment below says "get the aliases", but what the function really
returns
is the identifiers used for creating aliases. Please correct the comment.
+/*
+ * Get the relation and column alias for a given Var node, which belongs
to
+ *
On Tue, Nov 22, 2016 at 6:24 PM, Etsuro Fujita
wrote:
> Hi Rushabh,
>
> On 2016/11/22 19:05, Rushabh Lathia wrote:
>
>> I started reviewing the patch and here are few initial review points and
>> questions for you.
>>
>
> Thanks for the review!
>
> 1)
>> -static void
>
>
>> There is a already a function to build targetlist for a given relation
>> build_tlist_to_deparse(), which does the same thing as this code for a
>> join or
>> base relation and when there are no local conditions. Why don't we use
>> that
>> function instead of duplicating that logic? If
Hi Rushabh,
On 2016/11/22 19:05, Rushabh Lathia wrote:
I started reviewing the patch and here are few initial review points and
questions for you.
Thanks for the review!
1)
-static void deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
+static void
On 2016/11/22 18:28, Ashutosh Bapat wrote:
The comments should explain why is the assertion true.
+/* Shouldn't be NIL */
+Assert(tlist != NIL);
+/* Should be same length */
+Assert(list_length(tlist) ==
list_length(foreignrel->reltarget->exprs));
Will revise.
I started reviewing the patch and here are few initial review points and
questions for you.
1)
-static void deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
+static void deparseExplicitTargetList(bool is_returning,
+ List *tlist,
+
The comments should explain why is the assertion true.
+/* Shouldn't be NIL */
+Assert(tlist != NIL);
+/* Should be same length */
+Assert(list_length(tlist) ==
list_length(foreignrel->reltarget->exprs));
>
> OK, I'd like to propose referencing to
On 2016/11/21 22:02, Ashutosh Bapat wrote:
You wrote:
Instead we should calculate tlist, the
first time we need it and then add it to the fpinfo.
I wrote:
Having said that, I agree on that point. I'd like to propose (1) adding a
new member to fpinfo, to store a list of output Vars from the
>
> Yeah, I modified the patch so, as I thought that would be consistent with
> the aggregate pushdown patch.
aggregate pushdown needs the tlist to judge the shippability of
targetlist. For joins that's not required, so we should defer, if we
can.
>
>>> Instead we should calculate tlist, the
>>>
On 2016/11/19 0:16, Ashutosh Bapat wrote:
Also another point
I guess, this note doesn't add much value in the given context.
Probably we should remove it.
+* Note: the tlist would have one-to-one correspondence with the
+* joining relation's reltarget->exprs because (1)
Also another point
I guess, this note doesn't add much value in the given context.
Probably we should remove it.
+* Note: the tlist would have one-to-one correspondence with the
+* joining relation's reltarget->exprs because (1) the above test
+* on PHVs
>
> /*
> * For a join relation or an upper relation, use
> deparseExplicitTargetList.
> * Likewise, for a base relation that is being deparsed as a subquery,
> in
> * which case the caller would have passed tlist that is non-NIL, use
> that
> * function. Otherwise, use
On 2016/11/16 16:38, Etsuro Fujita wrote:
On 2016/11/16 13:10, Ashutosh Bapat wrote:
I don't see any reason why DML/UPDATE pushdown should depend upon
subquery deparsing or least PHV patch. Combined together they can help
in more cases, but without those patches, we will be able to push-down
On 2016/11/16 18:14, Ashutosh Bapat wrote:
(1) You added the
following comments to deparseSelectSql:
+ /*
+* For a non-base relation, use the input tlist. If a base relation
is
+* being deparsed as a subquery, use input tlist, if the caller has
passed
+* one. The
Thanks.
> except for few things; (1) You added the
> following comments to deparseSelectSql:
>
> + /*
> +* For a non-base relation, use the input tlist. If a base relation
> is
> +* being deparsed as a subquery, use input tlist, if the caller has
> passed
> +* one.
On 2016/11/16 13:10, Ashutosh Bapat wrote:
On Wed, Nov 16, 2016 at 8:25 AM, Etsuro Fujita
wrote:
On 2016/11/15 19:04, Rushabh Lathia wrote:
Your latest patch doesn't not get apply cleanly apply on master branch.
Did you apply the patch set in [1]
On Wed, Nov 16, 2016 at 8:25 AM, Etsuro Fujita
wrote:
> On 2016/11/15 19:04, Rushabh Lathia wrote:
>>
>> Thanks Fujita-san for working on this. I've signed up to review this
>> patch.
>
>
> Thank you for reviewing the patch!
>
>> Your latest patch doesn't not get
On 2016/11/15 19:04, Rushabh Lathia wrote:
Thanks Fujita-san for working on this. I've signed up to review this patch.
Thank you for reviewing the patch!
Your latest patch doesn't not get apply cleanly apply on master branch.
Did you apply the patch set in [1]
On 2016/11/11 20:50, Etsuro Fujita wrote:
On 2016/11/11 19:22, Ashutosh Bapat wrote:
The patch looks in good shape now. Here are some comments. I have also
made several changes to comments correcting grammar, typos, style and
at few places logic. Let me know if the patch looks good.
OK, will
Thanks Fujita-san for working on this. I've signed up to review this patch.
Your latest patch doesn't not get apply cleanly apply on master branch.
patching file contrib/postgres_fdw/deparse.c
6 out of 17 hunks FAILED -- saving rejects to file
contrib/postgres_fdw/deparse.c.rej
patching file
On 2016/11/11 19:22, Ashutosh Bapat wrote:
The patch looks in good shape now.
Thanks for the review!
The patch looks in good shape now. Here are some comments. I have also
made several changes to comments correcting grammar, typos, style and
at few places logic. Let me know if the patch
On 2016/09/08 19:55, Etsuro Fujita wrote:
On 2016/09/07 13:21, Ashutosh Bapat wrote:
* with the patch:
postgres=# explain verbose delete from ft1 using ft2 where ft1.a =
ft2.a;
QUERY PLAN
On Mon, Nov 7, 2016 at 5:50 PM, Etsuro Fujita
wrote:
> On 2016/11/07 11:24, Etsuro Fujita wrote:
>>
>> On 2016/11/04 19:55, Etsuro Fujita wrote:
>>>
>>> Attached is an updated version of the patch.
>
>
>> I noticed that I have included an unrelated regression test in
On 2016/11/07 11:24, Etsuro Fujita wrote:
On 2016/11/04 19:55, Etsuro Fujita wrote:
Attached is an updated version of the patch.
I noticed that I have included an unrelated regression test in the
patch. Attached is a patch with the test removed.
I noticed that I inadvertently removed some
On 2016/11/04 19:55, Etsuro Fujita wrote:
Attached is an updated version of the patch.
I noticed that I have included an unrelated regression test in the
patch. Attached is a patch with the test removed.
Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/deparse.c
---
On 2016/11/04 13:08, Ashutosh Bapat wrote:
On Fri, Nov 4, 2016 at 9:19 AM, Etsuro Fujita
wrote:
On 2016/11/03 18:52, Ashutosh Bapat wrote:
I wrote:
Here is the updated version,
Other than the above issue and the alias issue we discussed, I addressed
all
your
On Fri, Nov 4, 2016 at 9:19 AM, Etsuro Fujita
wrote:
> On 2016/11/03 18:52, Ashutosh Bapat wrote:
>>>
>>> Here is the updated version, which includes the restructuring you
>>> proposed.
>>> Other than the above issue and the alias issue we discussed, I addressed
>>>
On 2016/11/03 18:52, Ashutosh Bapat wrote:
Here is the updated version, which includes the restructuring you proposed.
Other than the above issue and the alias issue we discussed, I addressed all
your comments except one on testing; I tried to add test cases where the
remote query is deparsed as
> Here is the updated version, which includes the restructuring you proposed.
> Other than the above issue and the alias issue we discussed, I addressed all
> your comments except one on testing; I tried to add test cases where the
> remote query is deparsed as nested subqueries, but I couldn't
On 2016/10/27 20:41, Etsuro Fujita wrote:
Here is the updated version, which includes the restructuring you
proposed. Other than the above issue and the alias issue we discussed,
I addressed all your comments except one on testing; I tried to add test
cases where the remote query is deparsed as
On 2016/10/27 18:06, Ashutosh Bapat wrote:
On Thu, Oct 27, 2016 at 12:46 PM, Etsuro Fujita
wrote:
On 2016/10/26 19:53, Ashutosh Bapat wrote:
On Wed, Oct 26, 2016 at 3:35 PM, Etsuro Fujita
My concern about your proposal is: it might not be worth complicating the
On Thu, Oct 27, 2016 at 12:46 PM, Etsuro Fujita
wrote:
> On 2016/10/26 19:53, Ashutosh Bapat wrote:
>>
>> On Wed, Oct 26, 2016 at 3:35 PM, Etsuro Fujita
>> wrote:
>
>
>>> In practice, the search cost would be negligible compared to the
On 2016/10/26 19:53, Ashutosh Bapat wrote:
On Wed, Oct 26, 2016 at 3:35 PM, Etsuro Fujita
wrote:
In practice, the search cost would be negligible compared to the cost of
explaining/executing the query.
My concern about your proposal is: it might not be worth
On Wed, Oct 26, 2016 at 3:35 PM, Etsuro Fujita
wrote:
> On 2016/10/26 18:25, Ashutosh Bapat wrote:
>
>> Your patch calls isSubqueryExpr() recursively for every Var in the
>> targetlist, which can be many for foreign tables with many columns.
>> For every such Var it
On 2016/10/26 18:25, Ashutosh Bapat wrote:
Your patch calls isSubqueryExpr() recursively for every Var in the
targetlist, which can be many for foreign tables with many columns.
For every such Var it may need to reach upto the relation which is
converted into subquery, which can as bad as
>
I guess, the arrays need to be
computed only once for any relation when the query for that relation
is deparsed the first time.
>
>
>>> Does this algorithm extend to the case where we consider paths for every
>>> join order?
>
>
>> Yes, if we store the information about which of
On 2016/10/26 16:11, Ashutosh Bapat wrote:
You wrote:
For
example, let's assume that a relation (1, 2, 3) is required to be
deparsed as subquery for an immediate upper relation (1, 2, 3, 4, 5)
(thus the other joining relation being (4,5)). While deparsing for
relation (1,2,3,4,5), the context
>
>
>> For every relation that is deparsed as a subquery, we will create a
>> separate context. Each such context will have an array described
>> above. This array will contain the targetlist and aliases for all the
>> relations, covered by that relation, that are required to be deparsed
>> as
On 2016/10/25 18:58, Ashutosh Bapat wrote:
You wrote:
13. The comment below is missing the main purpose i.e. creating a a unique
alias, in case the relation gets converted into a subquery. Lowest or
highest
relid will create a unique alias at given level of join and that would be
more
future
>
>> 13. The comment below is missing the main purpose i.e. creating a a unique
>> alias, in case the relation gets converted into a subquery. Lowest or
>> highest
>> relid will create a unique alias at given level of join and that would be
>> more
>> future proof. If we decide to consider paths
On 2016/10/22 17:19, Ashutosh Bapat wrote:
Review for postgres-fdw-more-full-join-pushdown-v6 patch.
The patch applies cleanly and regression is clean (make check in
regress directory and that in postgres_fdw).
Thanks for the review!
Here are some comments.
1. Because of the following code
> 11. I have reworded following comment and restructured the code that follows
> it
> in the attached patch.
> +/*
> + * Set the subquery information. If the relation performs a full outer
> + * join and if the input relations have non-NIL remote_conds, the input
> + * relations
Review for postgres-fdw-more-full-join-pushdown-v6 patch.
The patch applies cleanly and regression is clean (make check in
regress directory and that in postgres_fdw).
Here are some comments.
1. Because of the following code change, for a joinrel we might end up using
attrs_used, which will be
On 2016/09/28 18:35, Etsuro Fujita wrote:
Attached is an updated version of the patch.
I found a minor bug in that patch; the relation_index added to
PgFdwRelationInfo was defined as Index, but I used the modifier %d to
print that. So, I changed the data type to int. Also, I added a bit
On 2016/09/26 16:30, Etsuro Fujita wrote:
On 2016/09/13 14:17, Ashutosh Bapat wrote:
It won't remain minimal as the number of paths created increases,
increasing the number of times a query is deparsed. We deparse query
every time time we cost a path for a relation with use_remote_estimates
On 2016/09/27 13:33, Ashutosh Bapat wrote:
I wrote:
ISTM that the use of the same RTI for subqueries in multi-levels in a
remote
SQL makes the SQL a bit difficult to read. How about using the
position
of
the join rel in join_rel_list, (more precisely, the position plus
On Tue, Sep 27, 2016 at 8:48 AM, Etsuro Fujita
wrote:
> On 2016/09/26 20:20, Ashutosh Bapat wrote:
>>
>> On Mon, Sep 26, 2016 at 4:06 PM, Etsuro Fujita
>> wrote:
>>>
>>> On 2016/09/26 18:06, Ashutosh Bapat wrote:
On Mon, Sep 26,
On 2016/09/26 20:20, Ashutosh Bapat wrote:
On Mon, Sep 26, 2016 at 4:06 PM, Etsuro Fujita
wrote:
On 2016/09/26 18:06, Ashutosh Bapat wrote:
On Mon, Sep 26, 2016 at 1:05 PM, Etsuro Fujita
wrote:
ISTM that the use of the same RTI for
On Mon, Sep 26, 2016 at 4:06 PM, Etsuro Fujita
wrote:
> On 2016/09/26 18:06, Ashutosh Bapat wrote:
>>
>> On Mon, Sep 26, 2016 at 1:05 PM, Etsuro Fujita
>> wrote:
>
>
>>> ISTM that the use of the same RTI for subqueries in multi-levels in
On 2016/09/26 18:06, Ashutosh Bapat wrote:
On Mon, Sep 26, 2016 at 1:05 PM, Etsuro Fujita
wrote:
ISTM that the use of the same RTI for subqueries in multi-levels in a remote
SQL makes the SQL a bit difficult to read. How about using the position of
the join rel
On Mon, Sep 26, 2016 at 1:05 PM, Etsuro Fujita
wrote:
> On 2016/09/15 15:29, Ashutosh Bapat wrote:
>>
>> On Wed, Sep 14, 2016 at 8:52 PM, Robert Haas
>> wrote:
>
>
>>> I'm not sure why it wouldn't work
>>> to just use the lowest RTI involved in
1 - 100 of 118 matches
Mail list logo