CAST from numeric(18,3) to numeric doesnt work, posgresql 13.3

=?UTF-8?B?SsOhbiBQZWNzxZFr?= <jan.pecsok@gmail.com> writes:

Where conversion from numeric (18,3) to numeric didnt work as expected.

Hmm, interesting. The cause of this is that coerce_type_typmod
supposes that casting to a -1 typmod is a total no-op. Which
is true so far as the run-time behavior is concerned, but not
if something inspects the exposed type of the expression.

The attached seems to be enough to fix it, though I wonder if
we've made the same assumption anywhere else.

Although this changes no existing regression test results,
I'm still a bit hesitant to back-patch it, because I am not
real sure that nothing out there is depending on the current
behavior (which has stood for decades). I think it'd be all
right to put into HEAD, and maybe it's not too late for v14.

regards, tom lane

On Thu, 5 Aug 2021 at 02:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hmm, interesting. The cause of this is that coerce_type_typmod
supposes that casting to a -1 typmod is a total no-op. Which
is true so far as the run-time behavior is concerned, but not
if something inspects the exposed type of the expression.

The attached seems to be enough to fix it, though I wonder if
we've made the same assumption anywhere else.

+1 for fixing this.

Although this changes no existing regression test results,
I'm still a bit hesitant to back-patch it, because I am not
real sure that nothing out there is depending on the current
behavior (which has stood for decades). I think it'd be all
right to put into HEAD, and maybe it's not too late for v14.

This isn't the first time someone has hit this [1]https://dba.stackexchange.com/questions/116218/surprising-results-for-data-types-with-type-modifier. The failing CTE
query there is quite interesting because if ::varchar[] with no typmod
wasn't a no-op, it could have been used to fix it. In fact, that's
what the error message and hint suggest doing.

So this fix is potentially more widely useful, though I couldn't say
whether anyone is relying on the current behaviour.

Regards,
Dean

[1]: https://dba.stackexchange.com/questions/116218/surprising-results-for-data-types-with-type-modifier

Dean Rasheed <dean.a.rasheed@gmail.com> writes:

On Thu, 5 Aug 2021 at 02:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Although this changes no existing regression test results,
I'm still a bit hesitant to back-patch it, because I am not
real sure that nothing out there is depending on the current
behavior (which has stood for decades). I think it'd be all
right to put into HEAD, and maybe it's not too late for v14.

This isn't the first time someone has hit this [1]. The failing CTE
query there is quite interesting because if ::varchar[] with no typmod
wasn't a no-op, it could have been used to fix it. In fact, that's
what the error message and hint suggest doing.

Hmm. Jan's example isn't that compelling, since you could always
create the table manually with the desired column type. However
the CTE application is more interesting, since there's no really
easy way to satisfy the type-matching rule there. (You could
cast to the more restrictive typmod, but that might be the Wrong
Thing for your query.)

So maybe we should back-patch, but I still feel nervous about that.
Anybody else have an opinion here?

With one eye on the calendar, I'm thinking that if we do decide
to back-patch we should wait a week, rather than shoving this in
just before a release wrap. OTOH, if it's v14 only it'd be better
to have it appear in beta3 rather than later. Maybe we should
push to v14 now, and consider back-patch in a few months if there
are not complaints about beta3?

regards, tom lane

On Thursday, August 5, 2021, Tom Lane <tgl@sss.pgh.pa.us> wrote:

So maybe we should back-patch, but I still feel nervous about that.
Anybody else have an opinion here?

With one eye on the calendar, I'm thinking that if we do decide
to back-patch we should wait a week, rather than shoving this in
just before a release wrap. OTOH, if it's v14 only it'd be better
to have it appear in beta3 rather than later. Maybe we should
push to v14 now, and consider back-patch in a few months if there
are not complaints about beta3?

Here is a recent newbie help request I fielded in Discord:

WITH RECURSIVE fizz_buzz (sequence, modulo_3, modulo_5) AS (
SELECT 1, CAST('' AS CHAR(4)), CAST('' AS CHAR(4))
UNION ALL
SELECT sequence + 1,
CASE WHEN MOD(sequence + 1, 3) = 0 THEN 'Fizz'
ELSE '' END,
CASE WHEN MOD(sequence + 1, 5) = 0 THEN 'Buzz'
ELSE '' END
FROM fizz_buzz
WHERE sequence < 100
)

SELECT
CASE WHEN CONCAT(modulo_3, modulo_5) = '' THEN sequence
ELSE CONCAT(modulo_3, modulo_5) END AS fizzbuzz
FROM fizz_buzz;

LINE 2: SELECT 1, CAST('' AS CHAR(4)), CAST('' AS CHAR(4))
^
HINT: Cast the output of the non-recursive term to the correct type.

And the just posted Bug 17137:

/messages/by-id/17137-3d3732d5a259612c@postgresql.org

Are these related?

I would agree with doing this in v14 then back-patch with the next update.
But i don’t have a technical feel here.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

Here is a recent newbie help request I fielded in Discord:

WITH RECURSIVE fizz_buzz (sequence, modulo_3, modulo_5) AS (
SELECT 1, CAST('' AS CHAR(4)), CAST('' AS CHAR(4))
UNION ALL
SELECT sequence + 1,
CASE WHEN MOD(sequence + 1, 3) = 0 THEN 'Fizz'
ELSE '' END,
CASE WHEN MOD(sequence + 1, 5) = 0 THEN 'Buzz'
ELSE '' END
FROM fizz_buzz
WHERE sequence < 100
)
SELECT
CASE WHEN CONCAT(modulo_3, modulo_5) = '' THEN sequence
ELSE CONCAT(modulo_3, modulo_5) END AS fizzbuzz
FROM fizz_buzz;

I'd class that one as pilot error, in that the query author
isn't even trying to make the two sides of the union agree
type-wise. It's not the same as the point at hand, where
someone did try to cast to the correct type but the parser
ignored it.

And the just posted Bug 17137:
/messages/by-id/17137-3d3732d5a259612c@postgresql.org
Are these related?

Right, that one is a dup of the current issue.

I would agree with doing this in v14 then back-patch with the next update.

Yeah, I'm going to push to v14 shortly if I don't hear objections soon.

regards, tom lane

A customer reported a planning regression from 11.5 to 12.9. After
bisecting, it seems the fix for the thread subject here, commit
f230614da28, broke partition pruning in some cases. Here's a
reproducer:

drop table if exists test_pruning;
create table test_pruning (account_id character(16) primary key)
partition by hash(account_id);
create table p0_test_pruning partition of test_pruning
for values with (modulus 5, remainder 0);
create table p1_test_pruning partition of test_pruning
for values with (modulus 5, remainder 1);
create table p2_test_pruning partition of test_pruning
for values with (modulus 5, remainder 2);
create table p3_test_pruning partition of test_pruning
for values with (modulus 5, remainder 3);
create table p4_test_pruning partition of test_pruning
for values with (modulus 5, remainder 4);

insert into test_pruning select 'XY' || lpad(i::text, 14, '0') from
generate_series(1,1000000,1) as i;

-- explicit cast on both operands
EXPLAIN (ANALYZE)
SELECT COUNT(*) FROM test_pruning WHERE account_id::BPCHAR =
'XY99999999999999'::BPCHAR;

-- explicit cast only on const
EXPLAIN (ANALYZE)
SELECT COUNT(*) FROM test_pruning WHERE account_id = 'XY99999999999999'::BPCHAR;

These queries both allowed partition pruning before, but with this
commit only the latter does:

QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------
Finalize Aggregate (cost=12159.67..12159.68 rows=1 width=8) (actual
time=66.891..68.641 rows=1 loops=1)
-> Gather (cost=12159.46..12159.67 rows=2 width=8) (actual
time=66.784..68.635 rows=3 loops=1)
Workers Planned: 2
Workers Launched: 2
-> Partial Aggregate (cost=11159.46..11159.47 rows=1
width=8) (actual time=56.140..56.142 rows=1 loops=3)
-> Parallel Append (cost=0.00..11156.64 rows=1128
width=0) (actual time=56.137..56.138 rows=0 loops=3)
-> Parallel Seq Scan on p2_test_pruning
(cost=0.00..2238.25 rows=320 width=0) (actual time=35.421..35.421
rows=0 loops=1)
Filter: ((account_id)::bpchar =
'XY99999999999999'::bpchar)
Rows Removed by Filter: 200799
-> Parallel Seq Scan on p3_test_pruning
(cost=0.00..2236.50 rows=319 width=0) (actual time=39.589..39.589
rows=0 loops=1)
Filter: ((account_id)::bpchar =
'XY99999999999999'::bpchar)
Rows Removed by Filter: 200523
-> Parallel Seq Scan on p4_test_pruning
(cost=0.00..2227.75 rows=318 width=0) (actual time=10.156..10.156
rows=0 loops=3)
Filter: ((account_id)::bpchar =
'XY99999999999999'::bpchar)
Rows Removed by Filter: 66599
-> Parallel Seq Scan on p0_test_pruning
(cost=0.00..2224.25 rows=318 width=0) (actual time=17.785..17.785
rows=0 loops=2)
Filter: ((account_id)::bpchar =
'XY99999999999999'::bpchar)
Rows Removed by Filter: 99715
-> Parallel Seq Scan on p1_test_pruning
(cost=0.00..2224.25 rows=318 width=0) (actual time=27.336..27.336
rows=0 loops=1)
Filter: ((account_id)::bpchar =
'XY99999999999999'::bpchar)
Rows Removed by Filter: 199452
Planning Time: 0.476 ms
Execution Time: 68.716 ms
(23 rows)

QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=8.44..8.45 rows=1 width=8) (actual time=0.016..0.016
rows=1 loops=1)
-> Index Only Scan using p2_test_pruning_pkey on p2_test_pruning
(cost=0.42..8.44 rows=1 width=0) (actual time=0.014..0.014 rows=0
loops=1)
Index Cond: (account_id = 'XY99999999999999'::bpchar)
Heap Fetches: 0
Planning Time: 0.071 ms
Execution Time: 0.038 ms
(6 rows)

--
John Naylor
EDB: http://www.enterprisedb.com

John Naylor <john.naylor@enterprisedb.com> writes:

A customer reported a planning regression from 11.5 to 12.9. After
bisecting, it seems the fix for the thread subject here, commit
f230614da28, broke partition pruning in some cases.

[ facepalm... ] I didn't consider the possibility that
find_typmod_coercion_function would find something ...
so we're getting an actual call of bpchar(), not just
a RelabelType.

regards, tom lane