Skip to content

Commit 5c056b0

Browse files
committed
Don't elide casting to typmod -1.
Casting a value that's already of a type with a specific typmod to an unspecified typmod doesn't do anything so far as run-time behavior is concerned. However, it really ought to change the exposed type of the expression to match. Up to now, coerce_type_typmod hasn't bothered with that, which creates gotchas in contexts such as recursive unions. If for example one side of the union is numeric(18,3), but it needs to be plain numeric to match the other side, there's no direct way to express that. This is easy enough to fix, by inserting a RelabelType to update the exposed type of the expression. However, it's a bit nervous-making to change this behavior, because it's stood for a really long time. (I strongly suspect that it's like this in part because the logic pre-dates the introduction of RelabelType in 7.0. The commit log message for 57b30e8 is interesting reading here.) As a compromise, we'll sneak the change into 14beta3, and consider back-patching to stable branches if no complaints emerge in the next three months. Discussion: https://postgr.es/m/CABNQVagu3bZGqiTjb31a8D5Od3fUMs7Oh3gmZMQZVHZ=uWWWfQ@mail.gmail.com
1 parent 2642df9 commit 5c056b0

File tree

3 files changed

+75
-9
lines changed

3 files changed

+75
-9
lines changed

src/backend/parser/parse_coerce.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -758,25 +758,33 @@ coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod,
758758
CoercionPathType pathtype;
759759
Oid funcId;
760760

761-
/*
762-
* A negative typmod is assumed to mean that no coercion is wanted. Also,
763-
* skip coercion if already done.
764-
*/
765-
if (targetTypMod < 0 || targetTypMod == exprTypmod(node))
761+
/* Skip coercion if already done */
762+
if (targetTypMod == exprTypmod(node))
766763
return node;
767764

765+
/* Suppress display of nested coercion steps */
766+
if (hideInputCoercion)
767+
hide_coercion_node(node);
768+
768769
pathtype = find_typmod_coercion_function(targetTypeId, &funcId);
769770

770771
if (pathtype != COERCION_PATH_NONE)
771772
{
772-
/* Suppress display of nested coercion steps */
773-
if (hideInputCoercion)
774-
hide_coercion_node(node);
775-
776773
node = build_coercion_expression(node, pathtype, funcId,
777774
targetTypeId, targetTypMod,
778775
ccontext, cformat, location);
779776
}
777+
else
778+
{
779+
/*
780+
* We don't need to perform any actual coercion step, but we should
781+
* apply a RelabelType to ensure that the expression exposes the
782+
* intended typmod.
783+
*/
784+
node = applyRelabelType(node, targetTypeId, targetTypMod,
785+
exprCollation(node),
786+
cformat, location, false);
787+
}
780788

781789
return node;
782790
}

src/test/regress/expected/expressions.out

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,43 @@ select count(*) from date_tbl
158158
13
159159
(1 row)
160160

161+
--
162+
-- Test parsing of a no-op cast to a type with unspecified typmod
163+
--
164+
begin;
165+
create table numeric_tbl (f1 numeric(18,3), f2 numeric);
166+
create view numeric_view as
167+
select
168+
f1, f1::numeric(16,4) as f1164, f1::numeric as f1n,
169+
f2, f2::numeric(16,4) as f2164, f2::numeric as f2n
170+
from numeric_tbl;
171+
\d+ numeric_view
172+
View "public.numeric_view"
173+
Column | Type | Collation | Nullable | Default | Storage | Description
174+
--------+---------------+-----------+----------+---------+---------+-------------
175+
f1 | numeric(18,3) | | | | main |
176+
f1164 | numeric(16,4) | | | | main |
177+
f1n | numeric | | | | main |
178+
f2 | numeric | | | | main |
179+
f2164 | numeric(16,4) | | | | main |
180+
f2n | numeric | | | | main |
181+
View definition:
182+
SELECT numeric_tbl.f1,
183+
numeric_tbl.f1::numeric(16,4) AS f1164,
184+
numeric_tbl.f1::numeric AS f1n,
185+
numeric_tbl.f2,
186+
numeric_tbl.f2::numeric(16,4) AS f2164,
187+
numeric_tbl.f2 AS f2n
188+
FROM numeric_tbl;
189+
190+
explain (verbose, costs off) select * from numeric_view;
191+
QUERY PLAN
192+
-------------------------------------------------------------------------------------------------------------------------------------------------------
193+
Seq Scan on public.numeric_tbl
194+
Output: numeric_tbl.f1, (numeric_tbl.f1)::numeric(16,4), (numeric_tbl.f1)::numeric, numeric_tbl.f2, (numeric_tbl.f2)::numeric(16,4), numeric_tbl.f2
195+
(2 rows)
196+
197+
rollback;
161198
--
162199
-- Tests for ScalarArrayOpExpr with a hashfn
163200
--

src/test/regress/sql/expressions.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ select count(*) from date_tbl
6666
select count(*) from date_tbl
6767
where f1 not between symmetric '1997-01-01' and '1998-01-01';
6868

69+
70+
--
71+
-- Test parsing of a no-op cast to a type with unspecified typmod
72+
--
73+
begin;
74+
75+
create table numeric_tbl (f1 numeric(18,3), f2 numeric);
76+
77+
create view numeric_view as
78+
select
79+
f1, f1::numeric(16,4) as f1164, f1::numeric as f1n,
80+
f2, f2::numeric(16,4) as f2164, f2::numeric as f2n
81+
from numeric_tbl;
82+
83+
\d+ numeric_view
84+
85+
explain (verbose, costs off) select * from numeric_view;
86+
87+
rollback;
88+
89+
6990
--
7091
-- Tests for ScalarArrayOpExpr with a hashfn
7192
--

0 commit comments

Comments
 (0)