Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
-Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Saturday, July 25, 2015 2:59 AM To: Kaigai Kouhei(海外 浩平) Cc: Etsuro Fujita; pgsql-hackers@postgresql.org Subject: ##freemail## Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it? On Thu, Jul 23, 2015 at 8:27 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: A dark side is, as discussed in this thread, complexity of EvalPlanQual. RefetchForeignRow() returns a tuple based on foreign table definition, on the other hands, whole-row var points a tuple based on fdw_scan_tlist if exists. An alternative host-only plan-node and relevant expression will be constructed towards the definition of base foreign-table. So, we need to transform the tuple to the layout based on foreign table definition if we allow fdw_scan_tlist with scanrelid 0. However, I'm skeptical whether this solution is valid for long term. Once we support to push down expensive expression in target-list to remote side, fdw_scan_tlist will contain expression node rather than simple Var node. In this case, it is not obvious to reproduce a tuple according to the foreign table definition from a record based on the fdw_scan_tlist. I don't think we can realistically make a decision that pushing down target list expressions to the remote side is forever off the table. Is the problem here that it's not *possible* for an FDW to do the right thing, or just that it might be difficult to code in practice? I'm fuzzy on why this isn't just a matter of having RefetchForeignRow() return a row with the correct tuple descriptor. RefetchForeignRow() does not take ForeignScanState argument that knows how remote data is represented on the local side if valid fdw_scan_tlist is configured. Do we have no facility to lookup ScanState object by scanrelid on execution time, don't it? On the other hands, I'm inclined to think FDW driver should provide alternative whole-row reference (according to the base foreign-table definition) if it has a valid fdw_scan_tlist It is more suitable on join pushdown cases, because the alternative subplan (to be executed instead of the remote query) assumes all the EPQ tuples follows base table definitions as usual. Is it not easy to inject a junk TLE to reference a whole-row variable based on the foreign table definition? Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
On Wed, Jul 22, 2015 at 8:24 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Indeed, this commit allows ForeignScan to have fdw_scan_tlist, even if scanrelid 0, however, I'm uncertain about its reason/intention. Does it a preparation for the upcoming target-list-pushdown?? I guess Tom would have to comment on whether it could be used for that purpose. I assume that omitting columns could be interesting for some FDWs, if nothing else. Indeed. As current postgres_fdw doing, FDW driver puts dummy NULLs on unreferenced columns for network optimization, however, it shall become unnecessary if we can change definition of the expected record-type of foreign-table. Its advantage is more human readable remote query and less CPU cycle for projection. A dark side is, as discussed in this thread, complexity of EvalPlanQual. RefetchForeignRow() returns a tuple based on foreign table definition, on the other hands, whole-row var points a tuple based on fdw_scan_tlist if exists. An alternative host-only plan-node and relevant expression will be constructed towards the definition of base foreign-table. So, we need to transform the tuple to the layout based on foreign table definition if we allow fdw_scan_tlist with scanrelid 0. However, I'm skeptical whether this solution is valid for long term. Once we support to push down expensive expression in target-list to remote side, fdw_scan_tlist will contain expression node rather than simple Var node. In this case, it is not obvious to reproduce a tuple according to the foreign table definition from a record based on the fdw_scan_tlist. So, I'm inclined to prohibit to set fdw_scan_tlist/custom_scan_tlist for actual scan node (scanrelid 0), at present. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
On Wed, Jul 22, 2015 at 8:24 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Indeed, this commit allows ForeignScan to have fdw_scan_tlist, even if scanrelid 0, however, I'm uncertain about its reason/intention. Does it a preparation for the upcoming target-list-pushdown?? I guess Tom would have to comment on whether it could be used for that purpose. I assume that omitting columns could be interesting for some FDWs, if nothing else. -- 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
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
Fujita-san, Sorry for my late response. The latest foreign-join pushdown patch allows fdw_scan_tlist to be set to a targetlist even for simple foreign table scans. However, since I think we assume that the test tuple of a foreign table for an EPQ testing, whether it may be copied from the whole-row var or returned by the RefetchForeignRow routine, has the rowtype declared for the foreign table, ISTM that EPQ testing doesn't work properly in such a case since that the targetlist and qual are adjusted to reference fdw_scan_tlist in such a case. Maybe I'm missing something though. Let me confirm step-by-step. For EPQ testing, whole-row-reference or RefetchForeignRow pulls a record with row-type compatible to the base foreign table. Then, this record is stored in the es_epqTuple[] indexed by the base relation. According to the previous discussion, I expect these tuples are re-checked by built-in execution plan, but equivalent to the sub-plan entirely pushed out to the remote side. Do we see the same assumption? If so, next step is enhancement of ExecScanFetch() to run the alternative built-in plans towards each es_epqTuple[] records, if given scanrelid==0. In this case, expression nodes adjusted to fdw_scan_tlist never called, so it should not lead any problems...? I don't understand custom scans/joins exactly, but I have a similar concern for the simple-custom-scan case too. In case of custom scan/join, it fetches a record using heap_fetch() identified by ctid, and saved to es_epqTuple[]. Then, EvalPlanQual() walks down the plan-tree. Once it appears a node of custom-join (scanrelid==0), it shall call the equivalent alternatives if possible, or calls ExecProcNode() towards the underlying nodes then re-construct its result according to the custom_scan_tlist definition. It does not look to me problematic. Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
Hi KaiGai-san, On 2015/07/22 16:44, Kouhei Kaigai wrote: The latest foreign-join pushdown patch allows fdw_scan_tlist to be set to a targetlist even for simple foreign table scans. However, since I think we assume that the test tuple of a foreign table for an EPQ testing, whether it may be copied from the whole-row var or returned by the RefetchForeignRow routine, has the rowtype declared for the foreign table, ISTM that EPQ testing doesn't work properly in such a case since that the targetlist and qual are adjusted to reference fdw_scan_tlist in such a case. Maybe I'm missing something though. Let me confirm step-by-step. For EPQ testing, whole-row-reference or RefetchForeignRow pulls a record with row-type compatible to the base foreign table. Then, this record is stored in the es_epqTuple[] indexed by the base relation. According to the previous discussion, I expect these tuples are re-checked by built-in execution plan, but equivalent to the sub-plan entirely pushed out to the remote side. Do we see the same assumption? No, what I'm concerned about is the case when scanrelid 0. If so, next step is enhancement of ExecScanFetch() to run the alternative built-in plans towards each es_epqTuple[] records, if given scanrelid==0. In this case, expression nodes adjusted to fdw_scan_tlist never called, so it should not lead any problems...? When scanrelid = 0, I think we should run the alternative plans in ExecScanFetch or somewhere, as you mentioned. I don't understand custom scans/joins exactly, but I have a similar concern for the simple-custom-scan case too. In case of custom scan/join, it fetches a record using heap_fetch() identified by ctid, and saved to es_epqTuple[]. Then, EvalPlanQual() walks down the plan-tree. Once it appears a node of custom-join (scanrelid==0), it shall call the equivalent alternatives if possible, or calls ExecProcNode() towards the underlying nodes then re-construct its result according to the custom_scan_tlist definition. It does not look to me problematic. Sorry, I don't understand what you mean. Maybe I have to learn more about custom scans/joins, but thanks for the explanation! Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
-Original Message- From: Etsuro Fujita [mailto:fujita.ets...@lab.ntt.co.jp] Sent: Wednesday, July 22, 2015 7:05 PM To: Kaigai Kouhei(海外 浩平) Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it? Hi KaiGai-san, On 2015/07/22 16:44, Kouhei Kaigai wrote: The latest foreign-join pushdown patch allows fdw_scan_tlist to be set to a targetlist even for simple foreign table scans. However, since I think we assume that the test tuple of a foreign table for an EPQ testing, whether it may be copied from the whole-row var or returned by the RefetchForeignRow routine, has the rowtype declared for the foreign table, ISTM that EPQ testing doesn't work properly in such a case since that the targetlist and qual are adjusted to reference fdw_scan_tlist in such a case. Maybe I'm missing something though. Let me confirm step-by-step. For EPQ testing, whole-row-reference or RefetchForeignRow pulls a record with row-type compatible to the base foreign table. Then, this record is stored in the es_epqTuple[] indexed by the base relation. According to the previous discussion, I expect these tuples are re-checked by built-in execution plan, but equivalent to the sub-plan entirely pushed out to the remote side. Do we see the same assumption? No, what I'm concerned about is the case when scanrelid 0. Hmm. if scanrelid 0, then fdw_scan_tlist should be NIL. I want to put Assert(scanrelid==0 || fdw_scan_tlist == NIL) just after the GetForeignPlan() in createplan.c. I'm curious why you tried to put valid fdw_scan_tlist for scanrelid 0. It's unusual. If so, next step is enhancement of ExecScanFetch() to run the alternative built-in plans towards each es_epqTuple[] records, if given scanrelid==0. In this case, expression nodes adjusted to fdw_scan_tlist never called, so it should not lead any problems...? When scanrelid = 0, I think we should run the alternative plans in ExecScanFetch or somewhere, as you mentioned. OK, I don't understand custom scans/joins exactly, but I have a similar concern for the simple-custom-scan case too. In case of custom scan/join, it fetches a record using heap_fetch() identified by ctid, and saved to es_epqTuple[]. Then, EvalPlanQual() walks down the plan-tree. Once it appears a node of custom-join (scanrelid==0), it shall call the equivalent alternatives if possible, or calls ExecProcNode() towards the underlying nodes then re-construct its result according to the custom_scan_tlist definition. It does not look to me problematic. Sorry, I don't understand what you mean. Maybe I have to learn more about custom scans/joins, but thanks for the explanation! -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
-Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas Sent: Wednesday, July 22, 2015 11:19 PM To: Kaigai Kouhei(海外 浩平) Cc: Etsuro Fujita; pgsql-hackers@postgresql.org Subject: Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it? On Wed, Jul 22, 2015 at 8:13 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: No, what I'm concerned about is the case when scanrelid 0. Hmm. if scanrelid 0, then fdw_scan_tlist should be NIL. I want to put Assert(scanrelid==0 || fdw_scan_tlist == NIL) just after the GetForeignPlan() in createplan.c. I'm curious why you tried to put valid fdw_scan_tlist for scanrelid 0. It's unusual. Allowing that was part of the point of Tom Lane's commit 1a8a4e5cde2b7755e11bde2ea7897bd650622d3e. See the second bullet point, after the comma. Indeed, this commit allows ForeignScan to have fdw_scan_tlist, even if scanrelid 0, however, I'm uncertain about its reason/intention. Does it a preparation for the upcoming target-list-pushdown?? Thanks, -- NEC Business Creation Division / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fdw_scan_tlist for foreign table scans breaks EPQ testing, doesn't it?
On Wed, Jul 22, 2015 at 8:13 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: No, what I'm concerned about is the case when scanrelid 0. Hmm. if scanrelid 0, then fdw_scan_tlist should be NIL. I want to put Assert(scanrelid==0 || fdw_scan_tlist == NIL) just after the GetForeignPlan() in createplan.c. I'm curious why you tried to put valid fdw_scan_tlist for scanrelid 0. It's unusual. Allowing that was part of the point of Tom Lane's commit 1a8a4e5cde2b7755e11bde2ea7897bd650622d3e. See the second bullet point, after the comma. -- 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
