From aee719f37e0a483123c3aeb7508db89eb3b05ae7 Mon Sep 17 00:00:00 2001 From: Shigeru Hanada Date: Thu, 24 Feb 2011 17:53:56 +0900 Subject: [PATCH] Fix double-free bug in postgrepsql_fdw. --- .../expected/postgresql_fdw.out | 21 +++++--- contrib/postgresql_fdw/postgresql_fdw.c | 53 ++++++++----------- contrib/postgresql_fdw/sql/postgresql_fdw.sql | 3 ++ 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/contrib/postgresql_fdw/expected/postgresql_fdw.out b/contrib/postgresql_fdw/expected/postgresql_fdw.out index bcc6a3ceca..cedac5c1e9 100644 --- a/contrib/postgresql_fdw/expected/postgresql_fdw.out +++ b/contrib/postgresql_fdw/expected/postgresql_fdw.out @@ -28,12 +28,10 @@ CREATE TABLE t2( ); COPY t2 FROM stdin; CREATE FOREIGN TABLE ft1 ( - c1 integer OPTIONS (colname 'invalid'), - c2 text OPTIONS (colname 'C2'), + c1 integer, + c2 text, c3 date ) SERVER loopback1 OPTIONS (relname 't1'); -ALTER FOREIGN TABLE ft1 ALTER COLUMN c1 OPTIONS (SET colname 'C1'); -ALTER FOREIGN TABLE ft1 ALTER COLUMN c2 OPTIONS (DROP colname); CREATE FOREIGN TABLE ft2 ( c1 integer, c2 text, @@ -56,10 +54,10 @@ ALTER USER MAPPING FOR PUBLIC SERVER loopback2 OPTIONS (DROP user); SELECT * FROM ft2 ORDER BY c1; -- ERROR ERROR: could not execute foreign query DETAIL: ERROR: relation "public.invalid" does not exist -LINE 1: SELECT c1, c2, c3 FROM public.invalid ft2 +LINE 1: SELECT c1, c2, c3 FROM public.invalid ^ -HINT: SELECT c1, c2, c3 FROM public.invalid ft2 +HINT: SELECT c1, c2, c3 FROM public.invalid ALTER FOREIGN TABLE ft2 OPTIONS (SET relname 't2'); SELECT * FROM ft2 ORDER BY c1; c1 | c2 | c3 @@ -69,6 +67,15 @@ SELECT * FROM ft2 ORDER BY c1; 13 | buz | 01-03-1970 (3 rows) +-- query with projection +SELECT c1 FROM ft1 ORDER BY c1; + c1 +---- + 1 + 2 + 3 +(3 rows) + -- join two foreign tables SELECT * FROM ft1 JOIN ft2 ON (ft1.c1 = ft2.c1) ORDER BY ft1.c1; c1 | c2 | c3 | c1 | c2 | c3 @@ -97,7 +104,7 @@ SELECT * FROM ft1 t1 LEFT JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY 1,2,3,4,5,6; -- WHERE clause push-down set client_min_messages = debug1; SELECT * FROM ft1 WHERE c1 = 1 AND c2 = lower('FOO') AND c3 < now(); -DEBUG: deparsed SQL is "SELECT C1, c2, c3 FROM public.t1 ft1 WHERE ((c1 = 1) AND (c2 = 'foo'::text))" +DEBUG: deparsed SQL is "SELECT C1, c2, c3 FROM public.t1 WHERE ((c1 = 1) AND (c2 = 'foo'::text))" c1 | c2 | c3 ----+-----+------------ 1 | foo | 01-01-1970 diff --git a/contrib/postgresql_fdw/postgresql_fdw.c b/contrib/postgresql_fdw/postgresql_fdw.c index 940ff1f8d0..20ecabfeaa 100644 --- a/contrib/postgresql_fdw/postgresql_fdw.c +++ b/contrib/postgresql_fdw/postgresql_fdw.c @@ -3,7 +3,7 @@ * postgresql_fdw.c * foreign-data wrapper for PostgreSQL * - * Portions Copyright (c) 1996-2011, PostgreSQL Global Development Group + * Portions Copyright (c) 2010-2011, PostgreSQL Global Development Group * * IDENTIFICATION * contrib/postgresql_fdw/postgresql_fdw.c @@ -55,11 +55,13 @@ static void pgReScanForeignScan(ForeignScanState *node); static void pgEndForeignScan(ForeignScanState *node); /* helper for deparsing a request into SQL statement */ -static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel, Cost *startup_cost, Cost *total_cost); +static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel, + Cost *startup_cost, Cost *total_cost); static bool is_foreign_qual(PlannerInfo *root, RelOptInfo *baserel, Expr *expr); static bool foreign_qual_walker(Node *node, void *context); -static char *deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel); +static char *deparseSql(Oid foreigntableid, PlannerInfo *root, + RelOptInfo *baserel); static void storeResult(TupleTableSlot *slot, PGresult *res, int rowno); @@ -82,9 +84,9 @@ static void cleanup_connection(ResourceReleasePhase phase, */ typedef struct PgsqlFdwExecutionState { - PGconn *conn; - PGresult *res; - int nextrow; + PGconn *conn; /* connection used for the scan */ + PGresult *res; /* result of the scan, held until the scan ends */ + int nextrow; /* row index to be returned in next fetch */ } PgsqlFdwExecutionState; @@ -351,6 +353,7 @@ deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) context = deparse_context_for("foreigntable", foreigntableid); /* Collect used columns from restrict information and target list */ + attr_used = list_union(attr_used, baserel->reltargetlist); foreach (lc, baserel->baserestrictinfo) { List *l; @@ -359,25 +362,20 @@ deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) l = pull_var_clause((Node *) ri->clause, PVC_RECURSE_PLACEHOLDERS); attr_used = list_union(attr_used, l); } - attr_used = list_union(attr_used, baserel->reltargetlist); /* deparse SELECT target list */ appendStringInfoString(&sql, "SELECT "); first = true; - for (attr = baserel->min_attr; attr < baserel->max_attr; attr++) + for (attr = 1; attr <= baserel->max_attr; attr++) { Var *var; - /* System columns cannot be retrieved from remote. */ - if (attr < 0) - continue; - /* Separete columns with comma. */ if (!first) appendStringInfoString(&sql, ", "); first = false; - /* Use "NULL" for column which isn't in reltargetlist. */ + /* Use "NULL" for unused columns.*/ foreach (lc, attr_used) { var = lfirst(lc); @@ -391,11 +389,6 @@ deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) else appendStringInfo(&sql, "NULL"); } - if (strcmp(sql.data, "SELECT ") == 0) - { - ereport(DEBUG1, (errmsg("targetlist is empty, retrieve all columns"))); - appendStringInfo(&sql, "*"); - } /* * Deparse FROM @@ -486,8 +479,6 @@ deparseSql(Oid foreigntableid, PlannerInfo *root, RelOptInfo *baserel) /* * Initiate actual scan on a foreign table. - * This function is called just after pgOpen() if the ForeignScan was executed - * for a real query or EXPLAIN statement with ANALYZE option. */ static void pgBeginForeignScan(ForeignScanState *node, int eflags) @@ -545,7 +536,6 @@ pgBeginForeignScan(ForeignScanState *node, int eflags) bool isvarlena; FmgrInfo func; - /* TODO: cache FmgrInfo to use it again after pgReOpen() */ /* TODO: send parameters in binary format rather than text */ getTypeOutputInfo(types[i], &out_func_oid, &isvarlena); fmgr_info(out_func_oid, &func); @@ -631,6 +621,9 @@ pgEndForeignScan(ForeignScanState *node) { PgsqlFdwExecutionState *festate = (PgsqlFdwExecutionState *) node->fdw_state; + if (festate == NULL) + return; + PQclear(festate->res); pfree(festate); } @@ -690,13 +683,15 @@ storeResult(TupleTableSlot *slot, PGresult *res, int rowno) j++; } - /* build the tuple and put it into the tuplestore. */ + /* + * Build the tuple and put it into the slot. + * We don't have to free the tuple explicitly because it's been + * allocated in the per-tuple context. + */ tuple = BuildTupleFromCStrings(attinmeta, values); - ExecStoreTuple(tuple, slot, InvalidBuffer, true); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); } - /* clean up and return the tuplestore */ - tuplestore_donestoring(tupstore); pfree(values); } @@ -1000,8 +995,7 @@ ReleaseConnection(PGconn *conn) #endif /* - * Clean the connection up via ResourceOwner when pgClose couldn't close the - * connection gracefully. + * Clean the connection up via ResourceOwner. */ static void cleanup_connection(ResourceReleasePhase phase, @@ -1011,10 +1005,7 @@ cleanup_connection(ResourceReleasePhase phase, { ConnCacheEntry *entry = (ConnCacheEntry *) arg; - /* - * If the transaction was committed, the connection has been closed via - * pgClose() and ReleaseConnection(). - */ + /* If the transaction was committed, don't close connections. */ if (isCommit) return; diff --git a/contrib/postgresql_fdw/sql/postgresql_fdw.sql b/contrib/postgresql_fdw/sql/postgresql_fdw.sql index 441a5b4457..fe97ac5a92 100644 --- a/contrib/postgresql_fdw/sql/postgresql_fdw.sql +++ b/contrib/postgresql_fdw/sql/postgresql_fdw.sql @@ -66,6 +66,9 @@ SELECT * FROM ft2 ORDER BY c1; -- ERROR ALTER FOREIGN TABLE ft2 OPTIONS (SET relname 't2'); SELECT * FROM ft2 ORDER BY c1; +-- query with projection +SELECT c1 FROM ft1 ORDER BY c1; + -- join two foreign tables SELECT * FROM ft1 JOIN ft2 ON (ft1.c1 = ft2.c1) ORDER BY ft1.c1; -- join itself -- 2.39.5