On Thu, Feb 18, 2016 at 4:52 PM, Ashutosh Bapat
wrote:
> If the list in the joining relation changes (may be because we appended
> parameterized conditions), we would be breaking links on all the upper
> relations in the join tree in an unpredictable manner. The
On Thu, Feb 18, 2016 at 3:48 PM, Etsuro Fujita
wrote:
> On 2016/02/16 16:40, Etsuro Fujita wrote:
>
>> On 2016/02/16 16:02, Ashutosh Bapat wrote:
>>
>>> On Tue, Feb 16, 2016 at 12:26 PM, Etsuro Fujita
>>>
On 2016/02/16 16:40, Etsuro Fujita wrote:
On 2016/02/16 16:02, Ashutosh Bapat wrote:
On Tue, Feb 16, 2016 at 12:26 PM, Etsuro Fujita
> wrote:
On 2016/02/16 15:22, Ashutosh Bapat wrote:
During join planning, the
On 2016/02/16 16:02, Ashutosh Bapat wrote:
On Tue, Feb 16, 2016 at 12:26 PM, Etsuro Fujita
> wrote:
On 2016/02/16 15:22, Ashutosh Bapat wrote:
During join planning, the planner tries multiple combinations of
On Tue, Feb 16, 2016 at 12:26 PM, Etsuro Fujita wrote:
> On 2016/02/16 15:22, Ashutosh Bapat wrote:
>
>> During join planning, the planner tries multiple combinations of joining
>> relations, thus the same base or join relation can be part of multiple
>> of
On 2016/02/16 15:22, Ashutosh Bapat wrote:
During join planning, the planner tries multiple combinations of joining
relations, thus the same base or join relation can be part of multiple
of combination. Hence remote_conds or joinclauses will get linked
multiple times as they are bidirectional
On 2016/02/15 21:33, Ashutosh Bapat wrote:
Here's patch with better way to fix it. I think while concatenating the
lists, we need to copy the lists being appended and in all the cases. If
we don't copy, a change in those lists can cause changes in the upward
linkages and thus lists of any higher
During join planning, the planner tries multiple combinations of joining
relations, thus the same base or join relation can be part of multiple of
combination. Hence remote_conds or joinclauses will get linked multiple
times as they are bidirectional lists, thus breaking linkages of previous
join
Thanks Fujita-san for bug report and the fix. Sorry for bug.
Here's patch with better way to fix it. I think while concatenating the
lists, we need to copy the lists being appended and in all the cases. If we
don't copy, a change in those lists can cause changes in the upward
linkages and thus
On 2016/02/10 4:16, Robert Haas wrote:
On Tue, Feb 9, 2016 at 8:39 AM, Ashutosh Bapat
wrote:
Thanks Jeevan for your review and comments. PFA the patch which fixes those.
Committed with a couple more small adjustments.
Thanks for working on this, Robert,
Here's patch to remove this declaration. The Assert next probably prevents
the warning for build with asserts. But both those lines are not needed.
On Wed, Feb 10, 2016 at 12:01 PM, Jeff Janes wrote:
> On Tue, Feb 9, 2016 at 11:16 AM, Robert Haas
>
On Wed, Feb 10, 2016 at 7:12 AM, Ashutosh Bapat
wrote:
> Here's patch to remove this declaration. The Assert next probably prevents
> the warning for build with asserts. But both those lines are not needed.
I like the Assert(), so I kept that and ditched the
On Tue, Feb 9, 2016 at 8:39 AM, Ashutosh Bapat
wrote:
> Thanks Jeevan for your review and comments. PFA the patch which fixes those.
Committed with a couple more small adjustments.
Woohoo, finally!
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The
Hi,
I have reviewed the patch and it looks good to me.
make/make install/make check is fine (when done without -Wall -Werror).
Here are few comments:
1.
With -Wall -Werror, I see couple of warnings:
postgres_fdw.c: In function ‘estimate_path_cost_size’:
postgres_fdw.c:2248:13: error: ‘run_cost’
Yay, finally!
Thanks.
On Wed, Feb 10, 2016 at 12:46 AM, Robert Haas wrote:
> On Tue, Feb 9, 2016 at 8:39 AM, Ashutosh Bapat
> wrote:
> > Thanks Jeevan for your review and comments. PFA the patch which fixes
> those.
>
> Committed with a
On Tue, Feb 9, 2016 at 11:16 AM, Robert Haas wrote:
> On Tue, Feb 9, 2016 at 8:39 AM, Ashutosh Bapat
> wrote:
>> Thanks Jeevan for your review and comments. PFA the patch which fixes those.
>
> Committed with a couple more small
On Mon, Feb 8, 2016 at 5:45 AM, Etsuro Fujita
wrote:
> Maybe my explanation was not correct, but I'm saying that the targertlist of
> the above outer_plan should be set to the fdw_scan_tlist, to avoid
> misbehavior.
Yeah, I think you're right. So in this hunk:
+
On 2016/02/05 17:50, Ashutosh Bapat wrote:
Btw, IIUC, I think the patch fails to adjust the targetlist of the
top plan created that way, to output the fdw_scan_tlist, as
discussed in [1] (ie, I think the attached patch is needed, which is
created on top of your patch
On Sat, Feb 6, 2016 at 12:46 AM, Ashutosh Bapat
wrote:
> Here it is rebased. Thanks for the pgindent run and committing core changes.
> I have to manage only one patch now :)
>
> pgindent is giving trouble with following two comments
>
> 2213 /* Run
On 2016/02/04 21:42, Ashutosh Bapat wrote:
* Is it safe to replace outerjoinpath with its fdw_outerpath the
following way? I think that if the join relation represented by
outerjoinpath has local conditions that can't be executed remotely,
we have to keep outerjoinpath in the
On Fri, Feb 5, 2016 at 4:23 AM, Ashutosh Bapat
wrote:
> I have corrected this in this set of patches. Also, I have included the
> change to build the join relation description while constructing fpinfo in
> the main patch since that avoids repeated building of the
On Fri, Feb 5, 2016 at 9:09 AM, Ashutosh Bapat
wrote:
>> Would it be nuts to set fdw_scan_tlist in all cases? Would the code
>> come out simpler than what we have now?
>
> PFA the patch that can be applied on v9 patches.
>
> If there is a whole-row reference for
> Btw, IIUC, I think the patch fails to adjust the targetlist of the top
> plan created that way, to output the fdw_scan_tlist, as discussed in [1]
> (ie, I think the attached patch is needed, which is created on top of your
> patch pg_fdw_join_v8.patch).
>
>
fdw_scan_tlist represents the output
Would it be nuts to set fdw_scan_tlist in all cases? Would the code
> come out simpler than what we have now?
>
>
PFA the patch that can be applied on v9 patches.
If there is a whole-row reference for base relation, instead of adding that
as an additional column deparseTargetList() creates a
On 2016/02/04 21:57, Ashutosh Bapat wrote:
One more: I think the following in postgresGetForeignJoinPaths() is
a good idea, but I think it's okay to just check whether
root->rowMarks is non-NIL, because that since we have rowmarks for
all base relations except the target, if we
On Thu, Feb 4, 2016 at 3:28 PM, Etsuro Fujita
wrote:
> On 2016/02/04 17:58, Etsuro Fujita wrote:
>
>> On 2016/02/04 8:00, Robert Haas wrote:
>>
>>> On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas
>>> wrote:
>>>
On Wed, Feb 3, 2016 at 12:08
>
>If so, my concern is, the helper function probably wouldn't
>> extend to the parameterized-foreign-join-path cases, though that
>> would work well for the unparameterized-foreign-join-path cases. We
>> don't support parameterized-foreign-join paths for 9.6?
>>
>
> If we do
On 2016/02/04 17:58, Etsuro Fujita wrote:
On 2016/02/04 8:00, Robert Haas wrote:
On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas
wrote:
On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
wrote:
PFA patches with naming conventions similar to
> * Is it safe to replace outerjoinpath with its fdw_outerpath the following
> way? I think that if the join relation represented by outerjoinpath has
> local conditions that can't be executed remotely, we have to keep
> outerjoinpath in the path tree; we will otherwise fail to execute the local
On Thu, Feb 4, 2016 at 11:55 AM, Ashutosh Bapat
wrote:
> Patches attached with the previous mail.
The core patch seemed to me to be in good shape now, so I committed
that. Not sure I'll be able to get to another read-through of the
main patch today.
--
Robert
On Thu, Feb 4, 2016 at 11:55 AM, Ashutosh Bapat
wrote:
> A query with FOR UPDATE/SHARE will be considered parallel unsafe in
> has_parallel_hazard_walker() and root->glob->parallelModeOK will be marked
> false. This implies that none of the base relations and
On Thu, Feb 4, 2016 at 4:30 AM, Robert Haas wrote:
> On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas wrote:
> > On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
> > wrote:
> >> PFA patches with naming conventions similar
On 2016/01/29 17:52, Ashutosh Bapat wrote:
On Fri, Jan 29, 2016 at 2:05 PM, Etsuro Fujita
> wrote:
On 2016/01/29 1:26, Ashutosh Bapat wrote:
Here is the summary of changes from the last set of patches
2.
On 2016/02/04 8:00, Robert Haas wrote:
On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas wrote:
On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
wrote:
PFA patches with naming conventions similar to previous ones.
pg_fdw_core_v7.patch: core
On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
wrote:
> The patch implements your algorithm to deparse a query as described in
> previous mail. The logic is largely coded in deparseFromExprForRel() and
> foreign_join_ok(). The later one pulls up the clauses from
On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
wrote:
> PFA patches with naming conventions similar to previous ones.
> pg_fdw_core_v7.patch: core changes
> pg_fdw_join_v7.patch: postgres_fdw changes for join pushdown
> pg_join_pd_v7.patch: combined patch for
On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas wrote:
> On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
> wrote:
>> PFA patches with naming conventions similar to previous ones.
>> pg_fdw_core_v7.patch: core changes
>> pg_fdw_join_v7.patch:
On Tue, Feb 2, 2016 at 5:18 AM, Robert Haas wrote:
> On Mon, Feb 1, 2016 at 8:27 AM, Ashutosh Bapat
> wrote:
> > Here are patches rebased on recent commit
> > cc592c48c58d9c1920f8e2063756dcbcce79e4dd. Renamed original
> deparseSelectSql
>
On Tue, Feb 2, 2016 at 11:21 AM, Ashutosh Bapat
wrote:
>> Why does deparseSelectStmtForRel change the order of the existing
>> arguments? I have no issue with adding new arguments as required, but
>> rearranging the existing argument order doesn't serve any
On Mon, Feb 1, 2016 at 8:27 AM, Ashutosh Bapat
wrote:
> Here are patches rebased on recent commit
> cc592c48c58d9c1920f8e2063756dcbcce79e4dd. Renamed original deparseSelectSql
> as deparseSelectSqlForBaseRel and added deparseSelectSqlForJoinRel to
> construct
On Mon, Feb 1, 2016 at 6:48 PM, Robert Haas wrote:
> More when I have a bit more time to look at this...
Why does deparseSelectStmtForRel change the order of the existing
arguments? I have no issue with adding new arguments as required, but
rearranging the existing
On Fri, Jan 29, 2016 at 3:46 AM, Ashutosh Bapat
wrote:
> PFA patch to move code to deparse SELECT statement into a function
> deparseSelectStmtForRel(). This code is duplicated in
> estimate_path_cost_size() and postgresGetForeignPlan(), so moving it into a
>
On Fri, Jan 29, 2016 at 2:05 PM, Etsuro Fujita
wrote:
> On 2016/01/29 1:26, Ashutosh Bapat wrote:
>
>> Here's an updated version of the previous patches, broken up like before
>>
>
> 2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below
>>
>
>
On Fri, Jan 29, 2016 at 9:51 AM, Robert Haas wrote:
> On Thu, Jan 28, 2016 at 11:26 AM, Ashutosh Bapat
> wrote:
> > 2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below
>
> This patch no longer quite applies because of
On 2016/01/29 1:26, Ashutosh Bapat wrote:
Here's an updated version of the previous patches, broken up like before
2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below
Here is the summary of changes from the last set of patches
2. Included fix for EvalPlanQual in
On Thu, Jan 28, 2016 at 11:26 AM, Ashutosh Bapat
wrote:
> 1. pg_fdw_core_v3.patch: changes in core - more description below
I've committed most of this patch, with some modifications. In
particular, I moved CachedPlanSource's hasForeignJoin flag to the
On Thu, Jan 28, 2016 at 11:26 AM, Ashutosh Bapat
wrote:
> 2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below
This patch no longer quite applies because of conflicts with one of
your other patches that I applied today (cf. commit
On Thu, Jan 21, 2016 at 8:36 AM, Ashutosh Bapat
wrote:
>> What this patch does to the naming and calling conventions in
>> deparse.c does not good. Previously, we had deparseTargetList().
>> Now, we sometimes use that and sometimes deparseAttrsUsed() for almost
On Thu, Jan 21, 2016 at 12:47 AM, Ashutosh Bapat
wrote:
>> Well, we kind of need to get it right, not just be guessing.
>>
>> It looks to me as though GetConnection() only uses the user ID as a
>> cache lookup key. What it's trying to do is ensure that if user A
> > In such a
> > case, which userid should be stored in UserMapping structure?It might
> look
> > like setting GetUserId() as userid in UserMapping is wise, but not
> really.
> > All the foreign tables might have different effective userids, each
> > different from GetUserId() and what
On Thu, Jan 21, 2016 at 3:03 AM, Robert Haas wrote:
> On Mon, Jan 18, 2016 at 6:47 AM, Ashutosh Bapat
> wrote:
> > 2. pg_fdw_join_v2.patch: postgres_fdw changes for supporting join
> pushdown
>
> The very first hunk of this patch contains
On Thu, Jan 21, 2016 at 2:07 AM, Robert Haas wrote:
>
> Well, we kind of need to get it right, not just be guessing.
>
> It looks to me as though GetConnection() only uses the user ID as a
> cache lookup key. What it's trying to do is ensure that if user A and
> user B
On Mon, Jan 18, 2016 at 6:47 AM, Ashutosh Bapat
wrote:
> 2. pg_fdw_join_v2.patch: postgres_fdw changes for supporting join pushdown
The very first hunk of this patch contains annoying whitespace
changes. Even if the result is what pgindent is going to do anyway,
On Wed, Jan 20, 2016 at 8:53 AM, Ashutosh Bapat
wrote:
> I missed the example plan cache revalidation patch in the previous mail.
> Sorry. Here it is.
I see. Yeah, I don't see a reason offhand why that wouldn't work.
But you need to update src/backend/nodes for
On Wed, Jan 20, 2016 at 8:50 AM, Ashutosh Bapat
wrote:
>> Third, I removed GetUserMappingById(). As mentioned in the email to
>> which I replied earlier, that doesn't actually produce the same result
>> as the way we're doing it now, and might leave the user ID
I missed the example plan cache revalidation patch in the previous mail.
Sorry. Here it is.
On Wed, Jan 20, 2016 at 7:20 PM, Ashutosh Bapat <
ashutosh.ba...@enterprisedb.com> wrote:
>
>
> On Wed, Jan 20, 2016 at 4:58 AM, Robert Haas
> wrote:
>
>> On Mon, Jan 18, 2016 at
On Wed, Aug 19, 2015 at 8:40 AM, Ashutosh Bapat
wrote:
> I started reviewing the other patches.
>
> In patch foreign_join_v16.patch, the user mapping structure being passed to
> GetConnection() is the one obtained from GetUserMappingById().
> GetUserMappingById()
On Mon, Jan 18, 2016 at 6:47 AM, Ashutosh Bapat
wrote:
> Thanks Thom for bringing it to my notice quickly. Sorry for the same.
>
> Here are the patches.
>
> 1. pg_fdw_core_v2.patch: changes in core related to user mapping handling,
> GUC
>
Hi All,
PFA patches for postgres_fdw join pushdown, taken care of all TODOs in my
last mail.
Here is the list of things that have been improved/added new as compared to
Hanada-san's previous patch at [1].
1. Condition handling for join
Patch in [1] allowed a foreign join to be pushed down if
On 18 January 2016 at 10:46, Ashutosh Bapat
wrote:
> Hi All,
> PFA patches for postgres_fdw join pushdown, taken care of all TODOs in my
> last mail.
>
> Here is the list of things that have been improved/added new as compared to
> Hanada-san's previous patch at
On 2016/01/18 19:46, Ashutosh Bapat wrote:
PFA patches for postgres_fdw join pushdown, taken care of all TODOs in
my last mail.
Here is the list of things that have been improved/added new as compared
to Hanada-san's previous patch at [1].
Great! Thank you for working on that! I'll review
On 2015/12/11 14:16, Ashutosh Bapat wrote:
On Thu, Dec 10, 2015 at 11:20 PM, Robert Haas > wrote:
On Tue, Dec 8, 2015 at 6:40 AM, Etsuro Fujita
>
wrote:
> IMO
On Fri, Dec 11, 2015 at 12:16 AM, Ashutosh Bapat
wrote:
>> +1.
>>
> I think there is still a lot functionality that is offered without
> EvalPlanQual fix.
Sure. But I think that the EvalPlanQual-related fixes might have some
impact on the overall design, and I
On Wed, Dec 2, 2015 at 6:45 AM, Ashutosh Bapat
wrote:
> It's been a long time since last patch on this thread was posted. I have
> started
> to work on supporting join pushdown for postgres_fdw. Attached please find
> three
> patches
> 1. pg_fdw_core.patch:
On Tue, Dec 8, 2015 at 6:40 AM, Etsuro Fujita
wrote:
> IMO I want to see the EvalPlanQual fix in the first version for 9.6.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list
On Thu, Dec 10, 2015 at 11:20 PM, Robert Haas wrote:
> On Tue, Dec 8, 2015 at 6:40 AM, Etsuro Fujita
> wrote:
> > IMO I want to see the EvalPlanQual fix in the first version for 9.6.
>
> +1.
>
> I think there is still a lot functionality that
On Fri, Dec 11, 2015 at 3:41 AM, Robert Haas wrote:
> On Wed, Dec 2, 2015 at 6:45 AM, Ashutosh Bapat
> wrote:
> > It's been a long time since last patch on this thread was posted. I have
> > started
> > to work on supporting join pushdown
On Thu, Dec 3, 2015 at 12:36 PM, Etsuro Fujita
wrote:
> Hi Ashutosh,
>
> On 2015/12/02 20:45, Ashutosh Bapat wrote:
>
>> It's been a long time since last patch on this thread was posted. I have
>> started
>> to work on supporting join pushdown for postgres_fdw.
>>
>
On 2015/12/08 17:27, Ashutosh Bapat wrote:
On Thu, Dec 3, 2015 at 12:36 PM, Etsuro Fujita
> wrote:
Generating paths
A join between two foreign relations is considered safe to push
Hi Ashutosh,
On 2015/12/02 20:45, Ashutosh Bapat wrote:
It's been a long time since last patch on this thread was posted. I have
started
to work on supporting join pushdown for postgres_fdw.
Thanks for the work!
Generating paths
A join between two foreign relations is
On Tue, Aug 4, 2015 at 2:20 PM, Shigeru Hanada shigeru.han...@gmail.com
wrote:
Hi Ashutosh,
Sorry for leaving the thread.
2015-07-20 16:09 GMT+09:00 Ashutosh Bapat ashutosh.ba...@enterprisedb.com
:
In find_user_mapping(), if the first cache search returns a valid tuple,
it
is checked
Hi Ashutosh,
Sorry for leaving the thread.
2015-07-20 16:09 GMT+09:00 Ashutosh Bapat ashutosh.ba...@enterprisedb.com:
In find_user_mapping(), if the first cache search returns a valid tuple, it
is checked twice for validity, un-necessarily. Instead if the first search
returns a valid tuple,
Hi Hanada-san,
I have reviewed usermapping patch and here are some comments.
The patch applies cleanly on Head and compiles without problem. The make
check in regression folder does not show any failure.
In find_user_mapping(), if the first cache search returns a valid tuple, it
is checked twice
Thank for your comments.
2015-05-21 23:11 GMT+09:00 Robert Haas robertmh...@gmail.com:
On Sat, May 16, 2015 at 9:04 AM, Shigeru Hanada
shigeru.han...@gmail.com wrote:
d) All relations must have the same effective user id
This check is done with userid stored in PgFdwRelationInfo, which is
On Sat, May 16, 2015 at 6:34 PM, Shigeru Hanada shigeru.han...@gmail.com
wrote:
Attached is the v15 patch of foreign join support for postgres_fdw.
This patch is based on current master, and having being removed some
hunks which are not essential.
And I wrote description of changes done by
Thanks for the comments.
2015/05/01 22:35、Robert Haas robertmh...@gmail.com のメール:
On Mon, Apr 27, 2015 at 5:07 AM, Shigeru Hanada
shigeru.han...@gmail.com wrote:
2015-04-27 11:00 GMT+09:00 Kouhei Kaigai kai...@ak.jp.nec.com:
Hanada-san, could you adjust your postgres_fdw patch according to
76 matches
Mail list logo