Thread: elog() error, trying CURENT OF with foreign table
While trying out CURRENT OF with foreign table, ending up with error.
Regards,
Rushabh Lathia
postgres=# select version();
version
-----------------------------------------------------------------------------------------------------------------
PostgreSQL 9.3devel on x86_64-unknown-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-3), 64-bit
(1 row)
-- Create exptension & database
postgres=# CREATE EXTENSION postgres_fdw;
CREATE EXTENSION
postgres=# create database regression;
CREATE DATABASE
-- Create foreign server
postgres=# CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
postgres-# OPTIONS (dbname 'regression');
CREATE SERVER
postgres=# CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
CREATE USER MAPPING
-- Create table into remote server
postgres=# \c regression
You are now connected to database "regression" as user "rushabh".
regression=# create table test ( a int );
CREATE TABLE
regression=# insert into test values ( 1 );
INSERT 0 1
-- Connect to local server and create test function
regression=# \c postgres
-- Create foreign table
postgres=# create foreign table test ( a int ) server loopback;
CREATE FOREIGN TABLE
postgres=# CREATE OR REPLACE FUNCTION taest_func_dblink2()
postgres-# RETURNS numeric
postgres-# AS $$
postgres$# DECLARE c CURSOR FOR SELECT a FROM test FOR UPDATE;
postgres$# v_i numeric;
postgres$# BEGIN
postgres$# OPEN c;
postgres$# FETCH c INTO v_i;
postgres$# UPDATE test SET a=50 WHERE CURRENT OF c;
postgres$# RETURN 0;
postgres$# END; $$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=# select taest_func_dblink2();
ERROR: CURRENT OF cannot be executed
CONTEXT: SQL statement "UPDATE test SET a=50 WHERE CURRENT OF c"
PL/pgSQL function taest_func_dblink2() line 7 at SQL statement
Here test ending up with following:
elog(ERROR, "CURRENT OF cannot be executed");
should we change this to ereport() or is there some other fix that we should make?
Rushabh Lathia
Rushabh Lathia <rushabh.lathia@gmail.com> writes:
> While trying out CURRENT OF with foreign table, ending up with error.
Yeah, that's an unimplemented feature.
In principle I think it could be made to work with postgres_fdw (since
that uses CTID row identification), but I doubt that it'd be possible
to promise that it works for every FDW.
regards, tom lane
On Fri, Apr 19, 2013 at 10:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Rushabh Lathia <rushabh.lathia@gmail.com> writes: >> While trying out CURRENT OF with foreign table, ending up with error. > > Yeah, that's an unimplemented feature. > > In principle I think it could be made to work with postgres_fdw (since > that uses CTID row identification), but I doubt that it'd be possible > to promise that it works for every FDW. So, should we just make that an ereport(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ...) instead of elog()? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Apr 19, 2013 at 10:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, that's an unimplemented feature.
> So, should we just make that an
> ereport(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ...) instead of
> elog()?
I'm not that excited about the errcode; if we're going to do anything,
changing the message text seems more important. Perhaps we could have
it say "WHERE CURRENT OF is not supported for this table type"? That's
jumping to conclusions about why the expression didn't get converted,
but at least for this case it'd be a more useful user-facing message.
regards, tom lane
On Fri, Apr 19, 2013 at 10:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Fri, Apr 19, 2013 at 10:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Yeah, that's an unimplemented feature. > >> So, should we just make that an >> ereport(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ...) instead of >> elog()? > > I'm not that excited about the errcode; if we're going to do anything, > changing the message text seems more important. Perhaps we could have > it say "WHERE CURRENT OF is not supported for this table type"? That's > jumping to conclusions about why the expression didn't get converted, > but at least for this case it'd be a more useful user-facing message. Yeah, it's probably good to improve the error message, too; and that suggestion seems as good as any. But I still think it should be ereport if it's user-facing. My main concern was actually whether we ought to be detecting this earlier in the process, before it gets as far as the executor. I haven't scrutinized the code though so have no particular reason to believe it's not OK as-is. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> My main concern was actually whether we ought to be detecting this
> earlier in the process, before it gets as far as the executor.
Yeah, that might be an appropriate response too. The executor is
coded so cavalierly because it expects the planner to have replaced
the CURRENT OF node with something executable. As things now stand,
whether that happens or not depends in part on the behavior of FDWs,
so maybe we'd better have the planner check whether it happened.
I'm not sure though if there's any suitably-painless place to do it.
regards, tom lane
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> My main concern was actually whether we ought to be detecting this
>> earlier in the process, before it gets as far as the executor.
> Yeah, that might be an appropriate response too. The executor is
> coded so cavalierly because it expects the planner to have replaced
> the CURRENT OF node with something executable. As things now stand,
> whether that happens or not depends in part on the behavior of FDWs,
> so maybe we'd better have the planner check whether it happened.
> I'm not sure though if there's any suitably-painless place to do it.
After looking at this a bit, my memory was faulty: the CurrentOfExpr
isn't really transformed to something else, we just make sure it ends up
in a TidScan node's tidqual list, where it won't be executed in the
normal way. The equivalent thing for a foreign table would be for the
FDW's execution code to have special smarts about what to do with a
CurrentOfExpr qual. So there's no way for the core planner to know
whether a ForeignScan incorporating a CurrentOfExpr qual is OK or not.
Short of a major redesign of the way this is handled, treating the
execution-time error as user-facing seems like the thing to do.
regards, tom lane