Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/09/21 0:40, Robert Haas wrote: On Fri, Jul 1, 2016 at 3:10 AM, Etsuro Fujita wrote: On 2016/04/14 4:57, Robert Haas wrote: 1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple before returning it from postgres_fdw, so that we don't expose

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Fri, Jul 1, 2016 at 3:10 AM, Etsuro Fujita wrote: > On 2016/04/14 4:57, Robert Haas wrote: >> >> 1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple >> before returning it from postgres_fdw, so that we don't expose the >> datum-tuple fields. I

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/04/14 4:57, Robert Haas wrote: 1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple before returning it from postgres_fdw, so that we don't expose the datum-tuple fields. I can't see any reason this isn't safe, but I might be missing something. I noticed odd behavior

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Fri, Apr 15, 2016 at 10:17 PM, Robert Haas wrote: > On Fri, Apr 15, 2016 at 12:16 PM, Ashutosh Bapat > wrote: > > The testcases had tableoid::regclass which outputs the foreign table's > local > > name, which won't change across runs.

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Fri, Apr 15, 2016 at 12:16 PM, Ashutosh Bapat wrote: > The testcases had tableoid::regclass which outputs the foreign table's local > name, which won't change across runs. Isn't that so? [rhaas pgsql]$ grep 16444 ~/Downloads/postgres-fdw-syscol-zap-ab.patch +

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Fri, Apr 15, 2016 at 9:39 PM, Robert Haas wrote: > On Thu, Apr 14, 2016 at 7:49 AM, Ashutosh Bapat > wrote: > > BTW, I noticed that we are deparsing whole-row reference as ROW(list of > > columns from local definition of foreign table),

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Thu, Apr 14, 2016 at 7:49 AM, Ashutosh Bapat wrote: > BTW, I noticed that we are deparsing whole-row reference as ROW(list of > columns from local definition of foreign table), which has the same problem > with outer joins. It won't be NULL when the rest of the

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/04/14 13:04, Robert Haas wrote: On Wed, Apr 13, 2016 at 11:21 PM, Etsuro Fujita wrote: 2. When a join is pushed down, deparse system columns using something like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID column, which gets deparsed

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

Thanks Robert for the patch. > OK, here's a patch. What I did is: > > 1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple > before returning it from postgres_fdw, so that we don't expose the > datum-tuple fields. I can't see any reason this isn't safe, but I > might be

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 11:21 PM, Etsuro Fujita wrote: >>> 2. When a join is pushed down, deparse system columns using something >>> like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID >>> column, which gets deparsed with the table OID in place of

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/04/14 12:04, Etsuro Fujita wrote: On 2016/04/14 4:57, Robert Haas wrote: 2. When a join is pushed down, deparse system columns using something like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID column, which gets deparsed with the table OID in place of 0. This

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/04/14 4:57, Robert Haas wrote: On Wed, Apr 13, 2016 at 2:36 PM, Robert Haas wrote: On Wed, Apr 13, 2016 at 2:11 PM, Robert Haas wrote: So, clearly that's not good. It should at least be consistent. But more than that, the fact that

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 5:33 PM, Tom Lane wrote: > Robert Haas writes: >> 2. When a join is pushed down, deparse system columns using something >> like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID >> column, which gets deparsed with

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

Robert Haas writes: > 2. When a join is pushed down, deparse system columns using something > like "CASE WHEN r1.* IS NOT NULL THEN 0 END", except for the table OID > column, which gets deparsed with the table OID in place of 0. This > delivers the correct behavior in the

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 2:36 PM, Robert Haas wrote: > On Wed, Apr 13, 2016 at 2:11 PM, Robert Haas wrote: >> So, clearly that's not good. It should at least be consistent. But >> more than that, the fact that postgres_fdw sets the xmax to

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 2:11 PM, Robert Haas wrote: > So, clearly that's not good. It should at least be consistent. But > more than that, the fact that postgres_fdw sets the xmax to 0x > is also pretty wacky. We might use such a value as a sentinel for > some

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

Robert Haas writes: > On Wed, Apr 13, 2016 at 1:36 PM, Robert Haas wrote: >> I tend to favor zeroes rather than NULLs, because that's what we >> typically use to represent an invalid value of those types, and I'm >> not aware of any current case

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 1:36 PM, Robert Haas wrote: > I tend to favor zeroes rather than NULLs, because that's what we > typically use to represent an invalid value of those types, and I'm > not aware of any current case where those values are NULL. In fact, see

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 13, 2016 at 1:36 PM, Robert Haas wrote: > I tend to favor zeroes rather than NULLs, because that's what we > typically use to represent an invalid value of those types, and I'm > not aware of any current case where those values are NULL. Actually, come to think

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Tue, Apr 5, 2016 at 4:54 AM, Ashutosh Bapat wrote: > With this patch, all instances of tableoid, cmin, cmax etc. will get a > non-NULL value irrespective of whether they appear on nullable side of the > join or not. > > e.g. select t1.c1, t1.tableoid, t2.c1,

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Apr 06, 2016 at 01:14:34AM -0400, Noah Misch wrote: > On Tue, Apr 05, 2016 at 03:41:00PM +0900, Etsuro Fujita wrote: > > On 2016/03/29 15:37, Etsuro Fujita wrote: > > >I added two helper functions: GetFdwScanTupleExtraData and > > >FillFdwScanTupleSysAttrs. The FDW author could use the

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Tue, Apr 05, 2016 at 03:41:00PM +0900, Etsuro Fujita wrote: > On 2016/03/29 15:37, Etsuro Fujita wrote: > >I added two helper functions: GetFdwScanTupleExtraData and > >FillFdwScanTupleSysAttrs. The FDW author could use the former to get > >info about system attributes other than ctids and

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

With this patch, all instances of tableoid, cmin, cmax etc. will get a non-NULL value irrespective of whether they appear on nullable side of the join or not. e.g. select t1.c1, t1.tableoid, t2.c1, t2.tableoid from ft4 t1 left join ft5 t2 on (t1.c1 = t2.c1); run in contrib_regression gives output

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/29 15:37, Etsuro Fujita wrote: I added two helper functions: GetFdwScanTupleExtraData and FillFdwScanTupleSysAttrs. The FDW author could use the former to get info about system attributes other than ctids and oids in fdw_scan_tlist during BeginForeignScan, and the latter to set

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/25 17:16, Etsuro Fujita wrote: The approach that we discussed would minimize the code for the FDW author to write, by providing the support functions you proposed. I'll post a patch for that early next week. I added two helper functions: GetFdwScanTupleExtraData and

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/25 13:37, Ashutosh Bapat wrote: A much simpler solution, that will work with postgres_fdw, might be to just deparse these columns with whatever random values (except for tableoid) they are expected to have in those places. Often these values can simply be NULL or 0. For tableoid

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

A much simpler solution, that will work with postgres_fdw, might be to just deparse these columns with whatever random values (except for tableoid) they are expected to have in those places. Often these values can simply be NULL or 0. For tableoid deparse it to 'oid value'::oid. Thus for a user

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Thu, Mar 24, 2016 at 9:31 AM, Etsuro Fujita wrote: > On 2016/03/23 13:44, Ashutosh Bapat wrote: > >> An FDW can choose not to use those functions, so I don't see a >> connection between scan list having simple Vars and existence of those >> functions (actually a

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/23 13:44, Ashutosh Bapat wrote: An FDW can choose not to use those functions, so I don't see a connection between scan list having simple Vars and existence of those functions (actually a single one). But having those function would minimize the code that each FDW has to write, in

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Wed, Mar 23, 2016 at 8:20 AM, Etsuro Fujita wrote: > On 2016/03/22 21:10, Ashutosh Bapat wrote: > >> On Tue, Mar 22, 2016 at 5:05 PM, Etsuro Fujita >> > wrote: >> On 2016/03/22 14:54, Ashutosh

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/22 21:10, Ashutosh Bapat wrote: On Tue, Mar 22, 2016 at 5:05 PM, Etsuro Fujita > wrote: On 2016/03/22 14:54, Ashutosh Bapat wrote: Earlier in this mail chain, I suggested that the core should take

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Tue, Mar 22, 2016 at 5:05 PM, Etsuro Fujita wrote: > On 2016/03/22 14:54, Ashutosh Bapat wrote: > >> On Tue, Mar 22, 2016 at 8:03 AM, Etsuro Fujita >> > wrote: >> OK, I'll modify the patch so

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/22 14:54, Ashutosh Bapat wrote: On Tue, Mar 22, 2016 at 8:03 AM, Etsuro Fujita > wrote: OK, I'll modify the patch so that the join is pushed down even if any of xmins, xmaxs, cmins, and cmaxs are requested. Do

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Tue, Mar 22, 2016 at 8:03 AM, Etsuro Fujita wrote: > On 2016/03/19 4:51, Robert Haas wrote: > >> On Thu, Mar 17, 2016 at 7:00 AM, Etsuro Fujita >> wrote: >> >>> So, I'd like to propose: (1) when tableoids are >>> requested from the

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/19 4:51, Robert Haas wrote: On Thu, Mar 17, 2016 at 7:00 AM, Etsuro Fujita wrote: So, I'd like to propose: (1) when tableoids are requested from the remote server, postgres_fdw sets valid values for them locally, instead (core should support that?)

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Thu, Mar 17, 2016 at 4:30 PM, Etsuro Fujita wrote: > Hi, > > I noticed that the postgres_fdw join pushdown patch retrieves system > columns other than ctid (and oid) from the remote server as shown in the > example: > > postgres=# explain verbose select

[HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

Hi, I noticed that the postgres_fdw join pushdown patch retrieves system columns other than ctid (and oid) from the remote server as shown in the example: postgres=# explain verbose select foo.tableoid, foo.xmin, foo.cmin, foo.xmax, foo.cmax, foo.* from foo, bar where foo.a = bar.a; QUERY

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On Thu, Mar 17, 2016 at 7:00 AM, Etsuro Fujita wrote: > BUT: we don't make any effort to ensure that local and remote values > match, so system columns other than ctid and oid should not be retrieved > from the remote server. I agree. > So, I'd like to propose: (1)

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

(2) when any of >> xmins, xmaxs, cmins, and cmaxs are requested, postgres_fdw gives up >> pushing down foreign joins. (We might be able to set appropriate >> values >> for them locally the same way as for tableoids, but I'm not sure it's >> worth complicating the code.) I

Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

On 2016/03/17 22:15, Ashutosh Bapat wrote: On Thu, Mar 17, 2016 at 4:30 PM, Etsuro Fujita > wrote: BUT: we don't make any effort to ensure that local and remote values match, so system columns other than ctid and oid