8000 Fix interval_transform so it doesn't throw away non-no-op casts. · hackingwu/postgres@beae7d5 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit beae7d5

Browse files
tglsfdctglsfdc
committed
Fix interval_transform so it doesn't throw away non-no-op casts.
interval_transform() contained two separate bugs that caused it to sometimes mistakenly decide that a cast from interval to restricted interval is a no-op and throw it away. First, it was wrong to rely on dt.h's field type macros to have an ordering consistent with the field's significance; in one case they do not. This led to mistakenly treating YEAR as less significant than MONTH, so that a cast from INTERVAL MONTH to INTERVAL YEAR was incorrectly discarded. Second, fls(1<<k) produces k+1 not k, so comparing its output directly to SECOND was wrong. This led to supposing that a cast to INTERVAL MINUTE was really a cast to INTERVAL SECOND and so could be discarded. To fix, get rid of the use of fls(), and make a function based on intervaltypmodout to produce a field ID code adapted to the need here. Per bug #14479 from Piotr Stefaniak. Back-patch to 9.2 where transform functions were introduced, because this code was born broken. Discussion: https://postgr.es/m/20161227172307.10135.7747@wrigleys.postgresql.org
1 parent 3ba8bed commit beae7d5

File tree

3 files changed

+105
-29
lines changed

3 files changed

+105
-29
lines changed

‎src/backend/utils/adt/timestamp.c

Lines changed: 82 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,59 @@ intervaltypmodout(PG_FUNCTION_ARGS)
922922
PG_RETURN_CSTRING(res);
923923
}
924924

925+
/*
926+
* Given an interval typmod value, return a code for the least-significant
927+
* field that the typmod allows to be nonzero, for instance given
928+
* INTERVAL DAY TO HOUR we want to identify "hour".
929+
*
930+
* The results should be ordered by field significance, which means
931+
* we can't use the dt.h macros YEAR etc, because for some odd reason
932+
* they aren't ordered that way. Instead, arbitrarily represent
933+
* SECOND = 0, MINUTE = 1, HOUR = 2, DAY = 3, MONTH = 4, YEAR = 5.
934+
*/
935+
static int
936+
intervaltypmodleastfield(int32 typmod)
937+
{
938+
if (typmod < 0)
939+
return 0; /* SECOND */
940+
941+
switch (INTERVAL_RANGE(typmod))
942+
{
943+
case INTERVAL_MASK(YEAR):
944+
return 5; /* YEAR */
945+
case INTERVAL_MASK(MONTH):
946+
return 4; /* MONTH */
947+
case INTERVAL_MASK(DAY):
948+
return 3; /* DAY */
949+
case INTERVAL_MASK(HOUR):
950+
return 2; /* HOUR */
951+
case INTERVAL_MASK(MINUTE):
952+
return 1; /* MINUTE */
953+
case INTERVAL_MASK(SECOND):
954+
return 0; /* SECOND */
955+
case INTERVAL_MASK(YEAR) | INTERVAL_MASK(MONTH):
956+
return 4; /* MONTH */
957+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR):
958+
return 2; /* HOUR */
959+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
960+
return 1; /* MINUTE */
961+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
962+
return 0; /* SECOND */
963+
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
964+
return 1; /* MINUTE */
965+
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
966+
return 0; /* SECOND */
967+
case INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
968+
return 0; /* SECOND */
969+
case INTERVAL_FULL_RANGE:
970+
return 0; /* SECOND */
971+
default:
972+
elog(ERROR, "invalid INTERVAL typmod: 0x%x", typmod);
973+
break;
974+
}
975+
return 0; /* can't get here, but keep compiler quiet */
976+
}
977+
925978

926979
/* interval_transform()
927980
* Flatten superfluous calls to interval_scale(). The interval typmod is
@@ -943,39 +996,39 @@ interval_transform(PG_FUNCTION_ARGS)
943996
if (IsA(typmod, Const) &&!((Const *) typmod)->constisnull)
944997
{
945998
Node *source = (Node *) linitial(expr->args);
946-
int32 old_typmod = exprTypmod(source);
947999
int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue);
948-
int old_range;
949-
int old_precis;
950-
int new_range = INTERVAL_RANGE(new_typmod);
951-
int new_precis = INTERVAL_PRECISION(new_typmod);
952-
int new_range_fls;
953-
int old_range_fls;
954-
955-
if (old_typmod < 0)
956-
{
957-
old_range = INTERVAL_FULL_RANGE;
958-
old_precis = INTERVAL_FULL_PRECISION;
959-
}
1000+
bool noop;
1001+
1002+
if (new_typmod < 0)
1003+
noop = true;
9601004
else
9611005
{
962-
old_range = INTERVAL_RANGE(old_typmod);
963-
old_precis = INTERVAL_PRECISION(old_typmod);
964-
}
1006+
int32 old_typmod = exprTypmod(source);
1007+
int old_least_field;
1008+
int new_least_field;
1009+
int old_precis;
1010+
int new_precis;
1011+
1012+
old_least_field = intervaltypmodleastfield(old_typmod);
1013+
new_least_field = intervaltypmodleastfield(new_typmod);
1014+
if (old_typmod < 0)
1015+
old_precis = INTERVAL_FULL_PRECISION;
1016+
else
1017+
old_precis = INTERVAL_PRECISION(old_typmod);
1018+
new_precis = INTERVAL_PRECISION(new_typmod);
9651019

966-
/*
967-
* Temporally-smaller fields occupy higher positions in the range
968-
* bitmap. Since only the temporally-smallest bit matters for length
969-
* coercion purposes, we compare the last-set bits in the ranges.
970-
* Precision, which is to say, sub-second precision, only affects
971-
* ranges that include SECOND.
972-
*/
973-
new_range_fls = fls(new_range);
974-
old_range_fls = fls(old_range);
975-
if (new_typmod < 0 ||
976-
((new_range_fls >= SECOND || new_range_fls >= old_range_fls) &&
977-
(old_range_fls < SECOND || new_precis >= MAX_INTERVAL_PRECISION ||
978-
new_precis >= old_precis)))
1020+
/*
1021+
* Cast is a no-op if least field stays the same or decreases
1022+
* while precision stays the same or increases. But precision,
1023+
* which is to say, sub-second precision, only affects ranges that
1024+
* include SECOND.
1025+
*/
1026+
noop = (new_least_field <= old_least_field) &&
1027+
(old_least_field > 0 /* SECOND */ ||
1028+
new_precis >= MAX_INTERVAL_PRECISION ||
1029+
new_precis >= old_precis);
1030+
}
1031+
if (noop)
9791032
ret = relabel_to_typmod(source, new_typmod);
9801033
}
9811034

‎src/test/regress/expected/interval.out

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,24 @@ SELECT interval '1 2:03:04.5678' minute to second(2);
692692
1 day 02:03:04.57
693693
(1 row)
694694

695+
-- test casting to restricted precision (bug #14479)
696+
SELECT f1, f1::INTERVAL DAY TO MINUTE AS "minutes",
697+
(f1 + INTERVAL '1 month')::INTERVAL MONTH::INTERVAL YEAR AS "years"
698+
FROM interval_tbl;
699+
f1 | minutes | years
700+
-----------------+-----------------+----------
701+
00:01:00 | 00:01:00 | 00:00:00
702+
05:00:00 | 05:00:00 | 00:00:00
703+
10 days | 10 days | 00:00:00
704+
34 years | 34 years | 34 years
705+
3 mons | 3 mons | 00:00:00
706+
-00:00:14 | 00:00:00 | 00:00:00
707+
1 day 02:03:04 | 1 day 02:03:00 | 00:00:00
708+
6 years | 6 years | 6 years
709+
5 mons | 5 mons | 00:00:00
710+
5 mons 12:00:00 | 5 mons 12:00:00 | 00:00:00
711+
(10 rows)
712+
695713
-- test inputting and outputting SQL standard interval literals
696714
SET IntervalStyle TO sql_standard;
697715
SELECT interval '0' AS "zero",

‎src/test/regress/sql/interval.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,11 @@ SELECT interval '1 2.3456' minute to second(2);
198198
SELECT interval '1 2:03.5678' minute to second(2);
199199
SELECT interval '1 2:03:04.5678' minute to second(2);
200200

201+
-- test casting to restricted precision (bug #14479)
202+
SELECT f1, f1::INTERVAL DAY TO MINUTE AS "minutes",
203+
(f1 + INTERVAL '1 month')::INTERVAL MONTH::INTERVAL YEAR AS "years"
204+
FROM interval_tbl;
205+
201206
-- test inputting and outputting SQL standard interval literals
202207
SET IntervalStyle TO sql_standard;
203208
SELECT interval '0' AS "zero",

0 commit comments

Comments
 (0)
0