mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-03 21:47:20 +02:00
Fix old bug with coercing the result of a COLLATE expression.
There are hacks in parse_coerce.c to push down a requested coercion to below any CollateExpr that may appear. However, we did that even if the requested data type is non-collatable, leading to an invalid expression tree in which CollateExpr is applied to a non-collatable type. The fix is just to drop the CollateExpr altogether, reasoning that it's useless. This bug is ten years old, dating to the original addition of COLLATE support. The lack of field complaints suggests that there aren't a lot of user-visible consequences. We noticed the problem because it would trigger an assertion in DefineVirtualRelation if the invalid structure appears as an output column of a view; however, in a non-assert build, you don't see a crash just a (subtly incorrect) complaint about applying collation to a non-collatable type. I found that by putting the incorrect structure further down in a view, I could make a view definition that would fail dump/reload, per the added regression test case. But CollateExpr doesn't do anything at run-time, so this likely doesn't lead to any really exciting consequences. Per report from Yulin Pei. Back-patch to all supported branches. Discussion: https://postgr.es/m/HK0PR01MB22744393C474D503E16C8509F4709@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
This commit is contained in:
parent
48d4a8c88b
commit
8a7bd1e6cb
@ -94,6 +94,7 @@ coerce_to_target_type(ParseState *pstate, Node *expr, Oid exprtype,
|
|||||||
* *must* know that to avoid possibly calling hide_coercion_node on
|
* *must* know that to avoid possibly calling hide_coercion_node on
|
||||||
* something that wasn't generated by coerce_type. Note that if there are
|
* something that wasn't generated by coerce_type. Note that if there are
|
||||||
* multiple stacked CollateExprs, we just discard all but the topmost.
|
* multiple stacked CollateExprs, we just discard all but the topmost.
|
||||||
|
* Also, if the target type isn't collatable, we discard the CollateExpr.
|
||||||
*/
|
*/
|
||||||
origexpr = expr;
|
origexpr = expr;
|
||||||
while (expr && IsA(expr, CollateExpr))
|
while (expr && IsA(expr, CollateExpr))
|
||||||
@ -113,7 +114,7 @@ coerce_to_target_type(ParseState *pstate, Node *expr, Oid exprtype,
|
|||||||
ccontext, cformat, location,
|
ccontext, cformat, location,
|
||||||
(result != expr && !IsA(result, Const)));
|
(result != expr && !IsA(result, Const)));
|
||||||
|
|
||||||
if (expr != origexpr)
|
if (expr != origexpr && type_is_collatable(targettype))
|
||||||
{
|
{
|
||||||
/* Reinstall top CollateExpr */
|
/* Reinstall top CollateExpr */
|
||||||
CollateExpr *coll = (CollateExpr *) origexpr;
|
CollateExpr *coll = (CollateExpr *) origexpr;
|
||||||
@ -385,20 +386,26 @@ coerce_type(ParseState *pstate, Node *node,
|
|||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
* If we have a COLLATE clause, we have to push the coercion
|
* If we have a COLLATE clause, we have to push the coercion
|
||||||
* underneath the COLLATE. This is really ugly, but there is little
|
* underneath the COLLATE; or discard the COLLATE if the target type
|
||||||
* choice because the above hacks on Consts and Params wouldn't happen
|
* isn't collatable. This is really ugly, but there is little choice
|
||||||
|
* because the above hacks on Consts and Params wouldn't happen
|
||||||
* otherwise. This kluge has consequences in coerce_to_target_type.
|
* otherwise. This kluge has consequences in coerce_to_target_type.
|
||||||
*/
|
*/
|
||||||
CollateExpr *coll = (CollateExpr *) node;
|
CollateExpr *coll = (CollateExpr *) node;
|
||||||
CollateExpr *newcoll = makeNode(CollateExpr);
|
|
||||||
|
|
||||||
newcoll->arg = (Expr *)
|
result = coerce_type(pstate, (Node *) coll->arg,
|
||||||
coerce_type(pstate, (Node *) coll->arg,
|
inputTypeId, targetTypeId, targetTypeMod,
|
||||||
inputTypeId, targetTypeId, targetTypeMod,
|
ccontext, cformat, location);
|
||||||
ccontext, cformat, location);
|
if (type_is_collatable(targetTypeId))
|
||||||
newcoll->collOid = coll->collOid;
|
{
|
||||||
newcoll->location = coll->location;
|
CollateExpr *newcoll = makeNode(CollateExpr);
|
||||||
return (Node *) newcoll;
|
|
||||||
|
newcoll->arg = (Expr *) result;
|
||||||
|
newcoll->collOid = coll->collOid;
|
||||||
|
newcoll->location = coll->location;
|
||||||
|
result = (Node *) newcoll;
|
||||||
|
}
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
|
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
|
||||||
&funcId);
|
&funcId);
|
||||||
|
@ -688,13 +688,26 @@ SELECT collation for ((SELECT b FROM collate_test1 LIMIT 1));
|
|||||||
"C"
|
"C"
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
|
-- old bug with not dropping COLLATE when coercing to non-collatable type
|
||||||
|
CREATE VIEW collate_on_int AS
|
||||||
|
SELECT c1+1 AS c1p FROM
|
||||||
|
(SELECT ('4' COLLATE "C")::INT AS c1) ss;
|
||||||
|
\d+ collate_on_int
|
||||||
|
View "collate_tests.collate_on_int"
|
||||||
|
Column | Type | Collation | Nullable | Default | Storage | Description
|
||||||
|
--------+---------+-----------+----------+---------+---------+-------------
|
||||||
|
c1p | integer | | | | plain |
|
||||||
|
View definition:
|
||||||
|
SELECT ss.c1 + 1 AS c1p
|
||||||
|
FROM ( SELECT 4 AS c1) ss;
|
||||||
|
|
||||||
--
|
--
|
||||||
-- Clean up. Many of these table names will be re-used if the user is
|
-- Clean up. Many of these table names will be re-used if the user is
|
||||||
-- trying to run any platform-specific collation tests later, so we
|
-- trying to run any platform-specific collation tests later, so we
|
||||||
-- must get rid of them.
|
-- must get rid of them.
|
||||||
--
|
--
|
||||||
DROP SCHEMA collate_tests CASCADE;
|
DROP SCHEMA collate_tests CASCADE;
|
||||||
NOTICE: drop cascades to 18 other objects
|
NOTICE: drop cascades to 19 other objects
|
||||||
DETAIL: drop cascades to table collate_test1
|
DETAIL: drop cascades to table collate_test1
|
||||||
drop cascades to table collate_test_like
|
drop cascades to table collate_test_like
|
||||||
drop cascades to table collate_test2
|
drop cascades to table collate_test2
|
||||||
@ -713,3 +726,4 @@ drop cascades to table collate_test21
|
|||||||
drop cascades to table collate_test22
|
drop cascades to table collate_test22
|
||||||
drop cascades to collation mycoll2
|
drop cascades to collation mycoll2
|
||||||
drop cascades to table collate_test23
|
drop cascades to table collate_test23
|
||||||
|
drop cascades to view collate_on_int
|
||||||
|
@ -266,6 +266,12 @@ SELECT collation for ('foo'::text);
|
|||||||
SELECT collation for ((SELECT a FROM collate_test1 LIMIT 1)); -- non-collatable type - error
|
SELECT collation for ((SELECT a FROM collate_test1 LIMIT 1)); -- non-collatable type - error
|
||||||
SELECT collation for ((SELECT b FROM collate_test1 LIMIT 1));
|
SELECT collation for ((SELECT b FROM collate_test1 LIMIT 1));
|
||||||
|
|
||||||
|
-- old bug with not dropping COLLATE when coercing to non-collatable type
|
||||||
|
CREATE VIEW collate_on_int AS
|
||||||
|
SELECT c1+1 AS c1p FROM
|
||||||
|
(SELECT ('4' COLLATE "C")::INT AS c1) ss;
|
||||||
|
\d+ collate_on_int
|
||||||
|
|
||||||
|
|
||||||
--
|
--
|
||||||
-- Clean up. Many of these table names will be re-used if the user is
|
-- Clean up. Many of these table names will be re-used if the user is
|
||||||
|
Loading…
Reference in New Issue
Block a user