Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-13 Thread Etsuro Fujita
On 2015/05/13 6:22, Tom Lane wrote: I wrote: I did a very basic update of your postgres_fdw patch to test this with, and attach that so that you don't have to repeat the effort. I'm not sure whether we want to try to convert that into something committable. I'm afraid that the extra round

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-13 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/13 6:22, Tom Lane wrote: Of course, if we don't do that then we still have your original gripe about ctid not being correct in EvalPlanQual results. I poked at this a bit and it seems like we could arrange to pass ctid through even

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-13 Thread Etsuro Fujita
On 2015/05/13 3:24, Tom Lane wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/12 7:42, Tom Lane wrote: I don't much like the division of labor between LockForeignRow and FetchForeignRow. In principle that would lead to not one but two extra remote accesses per locked row in

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-12 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/12 7:42, Tom Lane wrote: I don't much like the division of labor between LockForeignRow and FetchForeignRow. In principle that would lead to not one but two extra remote accesses per locked row in SELECT FOR UPDATE, at least in the

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-12 Thread Tom Lane
I wrote: I did a very basic update of your postgres_fdw patch to test this with, and attach that so that you don't have to repeat the effort. I'm not sure whether we want to try to convert that into something committable. I'm afraid that the extra round trips involved in doing row locking

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-11 Thread Etsuro Fujita
On 2015/05/11 8:50, Tom Lane wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: [ EvalPlanQual-v6.patch ] I've started to study this in a little more detail, and I'm not terribly happy with some of the API decisions in it. Thanks for taking the time to review the patch! In

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-11 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/11 8:50, Tom Lane wrote: In particular, I find the addition of void *fdw_state to ExecRowMark to be pretty questionable. That does not seem like a good place to keep random state. (I realize that WHERE CURRENT OF keeps some state

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-11 Thread Tom Lane
I wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/05/11 8:50, Tom Lane wrote: I wonder if we should instead add a ScanState* field and expect the core code to set that up (ExecOpenScanRelation could do it with minor API changes...). Sorry, I don't understand clearly what

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-11 Thread Tom Lane
On further consideration ... I don't much like the division of labor between LockForeignRow and FetchForeignRow. In principle that would lead to not one but two extra remote accesses per locked row in SELECT FOR UPDATE, at least in the case that an EvalPlanQual recheck is required. (I see that

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-11 Thread Etsuro Fujita
On 2015/05/12 7:42, Tom Lane wrote: On further consideration ... Thanks for the consideration! I don't much like the division of labor between LockForeignRow and FetchForeignRow. In principle that would lead to not one but two extra remote accesses per locked row in SELECT FOR UPDATE, at

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-10 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: [ EvalPlanQual-v6.patch ] I've started to study this in a little more detail, and I'm not terribly happy with some of the API decisions in it. In particular, I find the addition of void *fdw_state to ExecRowMark to be pretty questionable. That

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-05-07 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: BTW, I revised docs a bit. Attached is an updated version of the patch. I started to look at this and realized that it only touches the core code and not postgres_fdw, which seems odd --- doesn't that mean the new feature can't be tested?

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-30 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: Tom, you're listed as the committer for this in the CF app. Are you still planning to take care of this? It seems that time is beginning to run short. Yeah, I will address this (and start looking at GROUPING SETS) next week. I'm out of town right now.

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-30 Thread Robert Haas
On Thu, Apr 16, 2015 at 2:55 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Ah, you are right. FOR NO KEY UPDATE and FOR KEY SHARE would be useful in the Postgres FDW if we assume the user performs those properly based on information about keys for a remote table. Sorry, my explanation

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-16 Thread Etsuro Fujita
On 2015/04/15 2:27, Jim Nasby wrote: On 4/14/15 1:05 AM, Kyotaro HORIGUCHI wrote: As an example, the following operations cause an unexpected result. Those results are indeed surprising, but since we allow it in a direct connection I don't see why we wouldn't allow it in the Postgres FDW...

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-14 Thread Kyotaro HORIGUCHI
Hello, At Tue, 14 Apr 2015 12:10:35 +0900, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote in 552c852b.2050...@lab.ntt.co.jp On 2015/04/13 23:25, Jim Nasby wrote: On 4/13/15 4:58 AM, Etsuro Fujita wrote: On 2015/04/10 21:40, Etsuro Fujita wrote: On 2015/04/09 12:07, Etsuro Fujita wrote:

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-14 Thread Jim Nasby
On 4/14/15 1:05 AM, Kyotaro HORIGUCHI wrote: As an example, the following operations cause an unexpected result. Ex. 1 Session A=# create table t (a int primary key, b int); Session A=# insert into t (select a, 1 from generate_series(0, 99) a); Session A=# begin; Session A=# select * from t

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-13 Thread Etsuro Fujita
On 2015/04/13 23:25, Jim Nasby wrote: On 4/13/15 4:58 AM, Etsuro Fujita wrote: On 2015/04/10 21:40, Etsuro Fujita wrote: On 2015/04/09 12:07, Etsuro Fujita wrote: I'll update the patch, which will contain only an infrastructure for this in the PG core, and will not contain any postgres_fdw

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-13 Thread Etsuro Fujita
On 2015/04/10 21:40, Etsuro Fujita wrote: On 2015/04/09 12:07, Etsuro Fujita wrote: I'll update the patch, which will contain only an infrastructure for this in the PG core, and will not contain any postgres_fdw change. I updated the patch based on your comments. Updated patch attached. In

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-13 Thread Jim Nasby
On 4/13/15 4:58 AM, Etsuro Fujita wrote: On 2015/04/10 21:40, Etsuro Fujita wrote: On 2015/04/09 12:07, Etsuro Fujita wrote: I'll update the patch, which will contain only an infrastructure for this in the PG core, and will not contain any postgres_fdw change. I updated the patch based on

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-08 Thread Etsuro Fujita
On 2015/04/08 7:44, Tom Lane wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: To support ROW_MARK_REFERENCE on (postgres_fdw) foreign tables, I'd like to propose the following FDW APIs: RowMarkType GetForeignRowMarkType(Oid relid, LockClauseStrength

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-07 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: To support ROW_MARK_REFERENCE on (postgres_fdw) foreign tables, I'd like to propose the following FDW APIs: RowMarkType GetForeignRowMarkType(Oid relid, LockClauseStrength strength); Decide which rowmark type to use

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-03 Thread Etsuro Fujita
On 2015/03/25 4:56, Tom Lane wrote: Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: Let me explain further. Here is the comment in ExecOpenScanRelation: * Determine the lock type we need. First, scan to see if target relation * is a result relation. If not, check if it's

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-04-03 Thread Etsuro Fujita
On 2015/03/13 0:50, Tom Lane wrote: I think the real fix as far as postgres_fdw is concerned is in fact to let it adopt a different ROW_MARK strategy, since it has meaningful ctid values. However, that is not a one-size-fits-all answer. The tableoid problem can be fixed much less invasively

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-24 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: Let me explain further. Here is the comment in ExecOpenScanRelation: * Determine the lock type we need. First, scan to see if target relation * is a result relation. If not, check if it's a FOR UPDATE/FOR SHARE *

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-15 Thread Etsuro Fujita
On 2015/03/13 11:46, Etsuro Fujita wrote: BTW, what do you think about opening/locking foreign tables selected for update at InitPlan, which the original patch does? As I mentioned in [1], ISTM that ExecOpenScanRelation called from ExecInitForeignScan is assuming that. [1]

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-12 Thread Etsuro Fujita
On 2015/03/12 13:35, Tom Lane wrote: I don't like the execMain.c changes much at all. They look somewhat like they're intended to allow foreign tables to adopt a different locking strategy, I didn't intend such a thing. My intention is, foreign tables have markType = ROW_MARK_COPY as ever,

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-12 Thread Tom Lane
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: On 2015/03/12 13:35, Tom Lane wrote: I don't like the execMain.c changes much at all. They look somewhat like they're intended to allow foreign tables to adopt a different locking strategy, I didn't intend such a thing. My intention is,

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-11 Thread Ashutosh Bapat
Now I can reproduce the problem. Sanity Patch compiles cleanly and make check passes. The tests in file_fdw and postgres_fdw contrib modules pass. The patch works as expected in the test case reported. I have only one doubt. In EvalPlanQualFetchRowMarks(). tuple-t_self is assigned from

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-11 Thread Etsuro Fujita
On 2015/03/11 17:37, Ashutosh Bapat wrote: Now I can reproduce the problem. Sanity Patch compiles cleanly and make check passes. The tests in file_fdw and postgres_fdw contrib modules pass. The patch works as expected in the test case reported. Thanks for the testing! I have only

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-11 Thread Ashutosh Bapat
On Wed, Mar 11, 2015 at 5:10 PM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: On 2015/03/11 17:37, Ashutosh Bapat wrote: Now I can reproduce the problem. Sanity Patch compiles cleanly and make check passes. The tests in file_fdw and postgres_fdw contrib modules pass. The

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-11 Thread Tom Lane
Ashutosh Bapat ashutosh.ba...@enterprisedb.com writes: I will leave this issue for the committer to judge. Changed the status to ready for committer. I don't like the execMain.c changes much at all. They look somewhat like they're intended to allow foreign tables to adopt a different locking

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-09 Thread Etsuro Fujita
On 2015/03/09 16:02, Ashutosh Bapat wrote: I tried reproducing the issue with the steps summarised. Thanks for the review! Here's my setup Sorry, my explanation was not enough, but such was not my intention. I'll re-summarize the steps below: [Create a test environment] $ createdb

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-03-09 Thread Ashutosh Bapat
Hi Fujita-san, I tried reproducing the issue with the steps summarised. Here's my setup postgres=# \d ft Foreign table public.ft Column | Type | Modifiers | FDW Options +-+---+- a | integer | | Server: loopback FDW Options:

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-02-06 Thread Etsuro Fujita
Hi Ashutosh, On 2015/02/03 16:44, Ashutosh Bapat wrote: I am having some minor problems running this repro [Terminal 2] postgres=# create foreign table ft (a int) server loopback options (table_name 'lbt'); There isn't any table lbt mentioned here. Do you mean t here? Sorry,

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-02-02 Thread Ashutosh Bapat
Hi Fujita-san, I am having some minor problems running this repro On Thu, Jan 15, 2015 at 12:45 PM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Here is an example using postgres_fdw. [Terminal 1] postgres=# create table t (a int, b int); CREATE TABLE postgres=# insert into t values

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-01-28 Thread Etsuro Fujita
On 2015/01/19 17:10, Etsuro Fujita wrote: Attached is an updated version of the patch. I'll add this to the next CF. Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-01-19 Thread Etsuro Fujita
On 2015/01/16 1:24, Alvaro Herrera wrote: Etsuro Fujita wrote: *** 817,826 InitPlan(QueryDesc *queryDesc, int eflags) --- 818,833 break; case ROW_MARK_COPY: /* there's no real table here ... */ +

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

2015-01-15 Thread Alvaro Herrera
Etsuro Fujita wrote: *** *** 817,826 InitPlan(QueryDesc *queryDesc, int eflags) --- 818,833 break; case ROW_MARK_COPY: /* there's no real table here ... */ +