Fix contrib/postgres_fdw's remote-estimate representation of array Params.

We were emitting "(SELECT null::typename)", which is usually interpreted
as a scalar subselect, but not so much in the context "x = ANY(...)".
This led to remote-side parsing failures when remote_estimate is enabled.
A quick and ugly fix is to stick in an extra cast step,
"((SELECT null::typename)::typename)".  The cast will be thrown away as
redundant by parse analysis, but not before it's done its job of making
sure the grammar sees the ANY argument as an a_expr rather than a
select_with_parens.  Per an example from Hannu Krosing.
This commit is contained in:
Tom Lane 2014-04-16 17:21:57 -04:00
parent 9c53346500
commit 5b68d81697
3 changed files with 76 additions and 30 deletions

View File

@ -132,6 +132,10 @@ static void deparseRelabelType(RelabelType *node, deparse_expr_cxt *context);
static void deparseBoolExpr(BoolExpr *node, deparse_expr_cxt *context);
static void deparseNullTest(NullTest *node, deparse_expr_cxt *context);
static void deparseArrayExpr(ArrayExpr *node, deparse_expr_cxt *context);
static void printRemoteParam(int paramindex, Oid paramtype, int32 paramtypmod,
deparse_expr_cxt *context);
static void printRemotePlaceholder(Oid paramtype, int32 paramtypmod,
deparse_expr_cxt *context);
/*
@ -1283,16 +1287,11 @@ deparseVar(Var *node, deparse_expr_cxt *context)
*context->params_list = lappend(*context->params_list, node);
}
appendStringInfo(buf, "$%d", pindex);
appendStringInfo(buf, "::%s",
format_type_with_typemod(node->vartype,
node->vartypmod));
printRemoteParam(pindex, node->vartype, node->vartypmod, context);
}
else
{
appendStringInfo(buf, "(SELECT null::%s)",
format_type_with_typemod(node->vartype,
node->vartypmod));
printRemotePlaceholder(node->vartype, node->vartypmod, context);
}
}
}
@ -1399,26 +1398,12 @@ deparseConst(Const *node, deparse_expr_cxt *context)
*
* If we're generating the query "for real", add the Param to
* context->params_list if it's not already present, and then use its index
* in that list as the remote parameter number.
*
* If we're just generating the query for EXPLAIN, replace the Param with
* a dummy expression "(SELECT null::<type>)". In all extant versions of
* Postgres, the planner will see that as an unknown constant value, which is
* what we want. (If we sent a Param, recent versions might try to use the
* value supplied for the Param as an estimated or even constant value, which
* we don't want.) This might need adjustment if we ever make the planner
* flatten scalar subqueries.
*
* Note: we label the Param's type explicitly rather than relying on
* transmitting a numeric type OID in PQexecParams(). This allows us to
* avoid assuming that types have the same OIDs on the remote side as they
* do locally --- they need only have the same names.
* in that list as the remote parameter number. During EXPLAIN, there's
* no need to identify a parameter number.
*/
static void
deparseParam(Param *node, deparse_expr_cxt *context)
{
StringInfo buf = context->buf;
if (context->params_list)
{
int pindex = 0;
@ -1438,16 +1423,11 @@ deparseParam(Param *node, deparse_expr_cxt *context)
*context->params_list = lappend(*context->params_list, node);
}
appendStringInfo(buf, "$%d", pindex);
appendStringInfo(buf, "::%s",
format_type_with_typemod(node->paramtype,
node->paramtypmod));
printRemoteParam(pindex, node->paramtype, node->paramtypmod, context);
}
else
{
appendStringInfo(buf, "(SELECT null::%s)",
format_type_with_typemod(node->paramtype,
node->paramtypmod));
printRemotePlaceholder(node->paramtype, node->paramtypmod, context);
}
}
@ -1816,3 +1796,47 @@ deparseArrayExpr(ArrayExpr *node, deparse_expr_cxt *context)
appendStringInfo(buf, "::%s",
format_type_with_typemod(node->array_typeid, -1));
}
/*
* Print the representation of a parameter to be sent to the remote side.
*
* Note: we always label the Param's type explicitly rather than relying on
* transmitting a numeric type OID in PQexecParams(). This allows us to
* avoid assuming that types have the same OIDs on the remote side as they
* do locally --- they need only have the same names.
*/
static void
printRemoteParam(int paramindex, Oid paramtype, int32 paramtypmod,
deparse_expr_cxt *context)
{
StringInfo buf = context->buf;
char *ptypename = format_type_with_typemod(paramtype, paramtypmod);
appendStringInfo(buf, "$%d::%s", paramindex, ptypename);
}
/*
* Print the representation of a placeholder for a parameter that will be
* sent to the remote side at execution time.
*
* This is used when we're just trying to EXPLAIN the remote query.
* We don't have the actual value of the runtime parameter yet, and we don't
* want the remote planner to generate a plan that depends on such a value
* anyway. Thus, we can't do something simple like "$1::paramtype".
* Instead, we emit "((SELECT null::paramtype)::paramtype)".
* In all extant versions of Postgres, the planner will see that as an unknown
* constant value, which is what we want. This might need adjustment if we
* ever make the planner flatten scalar subqueries. Note: the reason for the
* apparently useless outer cast is to ensure that the representation as a
* whole will be parsed as an a_expr and not a select_with_parens; the latter
* would do the wrong thing in the context "x = ANY(...)".
*/
static void
printRemotePlaceholder(Oid paramtype, int32 paramtypmod,
deparse_expr_cxt *context)
{
StringInfo buf = context->buf;
char *ptypename = format_type_with_typemod(paramtype, paramtypmod);
appendStringInfo(buf, "((SELECT null::%s)::%s)", ptypename, ptypename);
}

View File

@ -592,6 +592,25 @@ WHERE a.c2 = 6 AND b.c1 = a.c1 AND a.c8 = 'foo' AND b.c7 = upper(a.c7);
996 | 6 | 00996 | Tue Apr 07 00:00:00 1970 PST | Tue Apr 07 00:00:00 1970 | 6 | 6 | foo | 996 | 6 | 00996 | Tue Apr 07 00:00:00 1970 PST | Tue Apr 07 00:00:00 1970 | 6 | 6 | foo
(100 rows)
-- bug before 9.3.5 due to sloppy handling of remote-estimate parameters
SELECT * FROM ft1 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft2 WHERE c1 < 5));
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
----+----+-------+------------------------------+--------------------------+----+------------+-----
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
2 | 2 | 00002 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
3 | 3 | 00003 | Sun Jan 04 00:00:00 1970 PST | Sun Jan 04 00:00:00 1970 | 3 | 3 | foo
4 | 4 | 00004 | Mon Jan 05 00:00:00 1970 PST | Mon Jan 05 00:00:00 1970 | 4 | 4 | foo
(4 rows)
SELECT * FROM ft2 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft1 WHERE c1 < 5));
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
----+----+-------+------------------------------+--------------------------+----+------------+-----
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
2 | 2 | 00002 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
3 | 3 | 00003 | Sun Jan 04 00:00:00 1970 PST | Sun Jan 04 00:00:00 1970 | 3 | 3 | foo
4 | 4 | 00004 | Mon Jan 05 00:00:00 1970 PST | Mon Jan 05 00:00:00 1970 | 4 | 4 | foo
(4 rows)
-- ===================================================================
-- parameterized queries
-- ===================================================================

View File

@ -200,6 +200,9 @@ EXPLAIN (VERBOSE, COSTS false)
WHERE a.c2 = 6 AND b.c1 = a.c1 AND a.c8 = 'foo' AND b.c7 = upper(a.c7);
SELECT * FROM ft2 a, ft2 b
WHERE a.c2 = 6 AND b.c1 = a.c1 AND a.c8 = 'foo' AND b.c7 = upper(a.c7);
-- bug before 9.3.5 due to sloppy handling of remote-estimate parameters
SELECT * FROM ft1 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft2 WHERE c1 < 5));
SELECT * FROM ft2 WHERE c1 = ANY (ARRAY(SELECT c1 FROM ft1 WHERE c1 < 5));
-- ===================================================================
-- parameterized queries