pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedGarbled comment in postgresGetForeignJoinPaths
9+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2017-08-17 b469387fc0 2a028de1ae 79f457e53a Remove bogus line from comment.
* Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 16:31 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 9+ messages in thread
From: Tom Lane @ 2017-08-16 16:31 UTC (permalink / raw)
To: pgsql-hackers
postgres_fdw.c around line 4500:
/*
* If there is a possibility that EvalPlanQual will be executed, we need
* to be able to reconstruct the row using scans of the base relations.
* GetExistingLocalJoinPath will find a suitable path for this purpose in
* the path list of the joinrel, if one exists. We must be careful to
* call it before adding any ForeignPath, since the ForeignPath might
* dominate the only suitable local path available. We also do it before
--> * reconstruct the row for EvalPlanQual(). Find an alternative local path
* calling foreign_join_ok(), since that function updates fpinfo and marks
* it as pushable if the join is found to be pushable.
*/
Should the marked line simply be deleted? If not, what correction is
appropriate?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 17:20 Robert Haas <robertmhaas@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 9+ messages in thread
From: Robert Haas @ 2017-08-16 17:20 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Wed, Aug 16, 2017 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> postgres_fdw.c around line 4500:
>
> /*
> * If there is a possibility that EvalPlanQual will be executed, we need
> * to be able to reconstruct the row using scans of the base relations.
> * GetExistingLocalJoinPath will find a suitable path for this purpose in
> * the path list of the joinrel, if one exists. We must be careful to
> * call it before adding any ForeignPath, since the ForeignPath might
> * dominate the only suitable local path available. We also do it before
> --> * reconstruct the row for EvalPlanQual(). Find an alternative local path
> * calling foreign_join_ok(), since that function updates fpinfo and marks
> * it as pushable if the join is found to be pushable.
> */
>
> Should the marked line simply be deleted? If not, what correction is
> appropriate?
Hmm, wow. My first thought was that it should just say
"reconstructing" rather than "reconstruct", but on further reading I
think you might have the right idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 18:16 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2017-08-16 18:16 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 16, 2017 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> --> * reconstruct the row for EvalPlanQual(). Find an alternative local path
>> Should the marked line simply be deleted? If not, what correction is
>> appropriate?
> Hmm, wow. My first thought was that it should just say
> "reconstructing" rather than "reconstruct", but on further reading I
> think you might have the right idea.
The current text of the comment dates to commit 177c56d60, and looking at
that commit makes it pretty clear that the line I'm complaining of
belonged to the previous text; it evidently just missed getting deleted.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 19:02 Robert Haas <robertmhaas@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Robert Haas @ 2017-08-16 19:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Wed, Aug 16, 2017 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Aug 16, 2017 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> --> * reconstruct the row for EvalPlanQual(). Find an alternative local path
>>> Should the marked line simply be deleted? If not, what correction is
>>> appropriate?
>
>> Hmm, wow. My first thought was that it should just say
>> "reconstructing" rather than "reconstruct", but on further reading I
>> think you might have the right idea.
>
> The current text of the comment dates to commit 177c56d60, and looking at
> that commit makes it pretty clear that the line I'm complaining of
> belonged to the previous text; it evidently just missed getting deleted.
Got it. Nice forensics, and sorry about the good.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 19:02 Robert Haas <robertmhaas@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Robert Haas @ 2017-08-16 19:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Wed, Aug 16, 2017 at 3:02 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Aug 16, 2017 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Wed, Aug 16, 2017 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> --> * reconstruct the row for EvalPlanQual(). Find an alternative local path
>>>> Should the marked line simply be deleted? If not, what correction is
>>>> appropriate?
>>
>>> Hmm, wow. My first thought was that it should just say
>>> "reconstructing" rather than "reconstruct", but on further reading I
>>> think you might have the right idea.
>>
>> The current text of the comment dates to commit 177c56d60, and looking at
>> that commit makes it pretty clear that the line I'm complaining of
>> belonged to the previous text; it evidently just missed getting deleted.
>
> Got it. Nice forensics, and sorry about the good.
... goof.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 19:44 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2017-08-16 19:44 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 16, 2017 at 3:02 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Wed, Aug 16, 2017 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The current text of the comment dates to commit 177c56d60, and looking at
>>> that commit makes it pretty clear that the line I'm complaining of
>>> belonged to the previous text; it evidently just missed getting deleted.
>> Got it. Nice forensics, and sorry about the good.
> ... goof.
Will you fix it, or shall I?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 19:46 Robert Haas <robertmhaas@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Robert Haas @ 2017-08-16 19:46 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Wed, Aug 16, 2017 at 3:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Aug 16, 2017 at 3:02 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Wed, Aug 16, 2017 at 2:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> The current text of the comment dates to commit 177c56d60, and looking at
>>>> that commit makes it pretty clear that the line I'm complaining of
>>>> belonged to the previous text; it evidently just missed getting deleted.
>
>>> Got it. Nice forensics, and sorry about the good.
>
>> ... goof.
>
> Will you fix it, or shall I?
Whichever you like.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-16 19:56 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: Tom Lane @ 2017-08-16 19:56 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 16, 2017 at 3:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Will you fix it, or shall I?
> Whichever you like.
It's your commit, you can do the honors.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Garbled comment in postgresGetForeignJoinPaths
@ 2017-08-17 12:02 Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 9+ messages in thread
From: Etsuro Fujita @ 2017-08-17 12:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers
On 2017/08/17 1:31, Tom Lane wrote:
> postgres_fdw.c around line 4500:
>
> /*
> * If there is a possibility that EvalPlanQual will be executed, we need
> * to be able to reconstruct the row using scans of the base relations.
> * GetExistingLocalJoinPath will find a suitable path for this purpose in
> * the path list of the joinrel, if one exists. We must be careful to
> * call it before adding any ForeignPath, since the ForeignPath might
> * dominate the only suitable local path available. We also do it before
> --> * reconstruct the row for EvalPlanQual(). Find an alternative local path
> * calling foreign_join_ok(), since that function updates fpinfo and marks
> * it as pushable if the join is found to be pushable.
> */
>
> Should the marked line simply be deleted? If not, what correction is
> appropriate?
In relation to this, I updated the patch for addressing the
GetExistingLocalJoinPath issue [1].
Best regards,
Etsuro Fujita
[1]
https://www.postgresql.org/message-id/6008ca34-906f-e61d-478b-8f85fde2c090%40lab.ntt.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2017-08-17 12:02 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 16:31 Garbled comment in postgresGetForeignJoinPaths Tom Lane <tgl@sss.pgh.pa.us>
2017-08-16 17:20 ` Robert Haas <robertmhaas@gmail.com>
2017-08-16 18:16 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-08-16 19:02 ` Robert Haas <robertmhaas@gmail.com>
2017-08-16 19:02 ` Robert Haas <robertmhaas@gmail.com>
2017-08-16 19:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-08-16 19:46 ` Robert Haas <robertmhaas@gmail.com>
2017-08-16 19:56 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-08-17 12:02 ` Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox