Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 21e24eb

Browse files
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 e9cf6e6 commit 21e24eb

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
@@ -1241,6 +1241,59 @@ intervaltypmodout(PG_FUNCTION_ARGS)
12411241
PG_RETURN_CSTRING(res);
12421242
}
12431243

1244+
/*
1245+
* Given an interval typmod value, return a code for the least-significant
1246+
* field that the typmod allows to be nonzero, for instance given
1247+
* INTERVAL DAY TO HOUR we want to identify "hour".
1248+
*
1249+
* The results should be ordered by field significance, which means
1250+
* we can't use the dt.h macros YEAR etc, because for some odd reason
1251+
* they aren't ordered that way. Instead, arbitrarily represent
1252+
* SECOND = 0, MINUTE = 1, HOUR = 2, DAY = 3, MONTH = 4, YEAR = 5.
1253+
*/
1254+
static int
1255+
intervaltypmodleastfield(int32 typmod)
1256+
{
1257+
if (typmod < 0)
1258+
return 0; /* SECOND */
1259+
1260+
switch (INTERVAL_RANGE(typmod))
1261+
{
1262+
case INTERVAL_MASK(YEAR):
1263+
return 5; /* YEAR */
1264+
case INTERVAL_MASK(MONTH):
1265+
return 4; /* MONTH */
1266+
case INTERVAL_MASK(DAY):
1267+
return 3; /* DAY */
1268+
case INTERVAL_MASK(HOUR):
1269+
return 2; /* HOUR */
1270+
case INTERVAL_MASK(MINUTE):
1271+
return 1; /* MINUTE */
1272+
case INTERVAL_MASK(SECOND):
1273+
return 0; /* SECOND */
1274+
case INTERVAL_MASK(YEAR) | INTERVAL_MASK(MONTH):
1275+
return 4; /* MONTH */
1276+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR):
1277+
return 2; /* HOUR */
1278+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
1279+
return 1; /* MINUTE */
1280+
case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
1281+
return 0; /* SECOND */
1282+
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
1283+
return 1; /* MINUTE */
1284+
case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
1285+
return 0; /* SECOND */
1286+
case INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
1287+
return 0; /* SECOND */
1288+
case INTERVAL_FULL_RANGE:
1289+
return 0; /* SECOND */
1290+
default:
1291+
elog(ERROR, "invalid INTERVAL typmod: 0x%x", typmod);
1292+
break;
1293+
}
1294+
return 0; /* can't get here, but keep compiler quiet */
1295+
}
1296+
12441297

12451298
/* interval_transform()
12461299
* Flatten superfluous calls to interval_scale(). The interval typmod is
@@ -1262,39 +1315,39 @@ interval_transform(PG_FUNCTION_ARGS)
12621315
if (IsA(typmod, Const) &&!((Const *) typmod)->constisnull)
12631316
{
12641317
Node *source = (Node *) linitial(expr->args);
1265-
int32 old_typmod = exprTypmod(source);
12661318
int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue);
1267-
int old_range;
1268-
int old_precis;
1269-
int new_range = INTERVAL_RANGE(new_typmod);
1270-
int new_precis = INTERVAL_PRECISION(new_typmod);
1271-
int new_range_fls;
1272-
int old_range_fls;
1273-
1274-
if (old_typmod < 0)
1275-
{
1276-
old_range = INTERVAL_FULL_RANGE;
1277-
old_precis = INTERVAL_FULL_PRECISION;
1278-
}
1319+
bool noop;
1320+
1321+
if (new_typmod < 0)
1322+
noop = true;
12791323
else
12801324
{
1281-
old_range = INTERVAL_RANGE(old_typmod);
1282-
old_precis = INTERVAL_PRECISION(old_typmod);
1283-
}
1325+
int32 old_typmod = exprTypmod(source);
1326+
int old_least_field;
1327+
int new_least_field;
1328+
int old_precis;
1329+
int new_precis;
1330+
1331+
old_least_field = intervaltypmodleastfield(old_typmod);
1332+
new_least_field = intervaltypmodleastfield(new_typmod);
1333+
if (old_typmod < 0)
1334+
old_precis = INTERVAL_FULL_PRECISION;
1335+
else
1336+
old_precis = INTERVAL_PRECISION(old_typmod);
1337+
new_precis = INTERVAL_PRECISION(new_typmod);
12841338

1285-
/*
1286-
* Temporally-smaller fields occupy higher positions in the range
1287-
* bitmap. Since only the temporally-smallest bit matters for length
1288-
* coercion purposes, we compare the last-set bits in the ranges.
1289-
* Precision, which is to say, sub-second precision, only affects
1290-
* ranges that include SECOND.
1291-
*/
1292-
new_range_fls = fls(new_range);
1293-
old_range_fls = fls(old_range);
1294-
if (new_typmod < 0 ||
1295-
((new_range_fls >= SECOND || new_range_fls >= old_range_fls) &&
1296-
(old_range_fls < SECOND || new_precis >= MAX_INTERVAL_PRECISION ||
1297-
new_precis >= old_precis)))
1339+
/*
1340+
* Cast is a no-op if least field stays the same or decreases
1341+
* while precision stays the same or increases. But precision,
1342+
* which is to say, sub-second precision, only affects ranges that
1343+
* include SECOND.
1344+
*/
1345+
noop = (new_least_field <= old_least_field) &&
1346+
(old_least_field > 0 /* SECOND */ ||
1347+
new_precis >= MAX_INTERVAL_PRECISION ||
1348+
new_precis >= old_precis);
1349+
}
1350+
if (noop)
12981351
ret = relabel_to_typmod(source, new_typmod);
12991352
}
13001353

src/test/regress/expected/interval.out

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

685+
-- test casting to restricted precision (bug #14479)
686+
SELECT f1, f1::INTERVAL DAY TO MINUTE AS "minutes",
687+
(f1 + INTERVAL '1 month')::INTERVAL MONTH::INTERVAL YEAR AS "years"
688+
FROM interval_tbl;
689+
f1 | minutes | years
690+
-----------------+-----------------+----------
691+
00:01:00 | 00:01:00 | 00:00:00
692+
05:00:00 | 05:00:00 | 00:00:00
693+
10 days | 10 days | 00:00:00
694+
34 years | 34 years | 34 years
695+
3 mons | 3 mons | 00:00:00
696+
-00:00:14 | 00:00:00 | 00:00:00
697+
1 day 02:03:04 | 1 day 02:03:00 | 00:00:00
698+
6 years | 6 years | 6 years
699+
5 mons | 5 mons | 00:00:00
700+
5 mons 12:00:00 | 5 mons 12:00:00 | 00:00:00
701+
(10 rows)
702+
685703
-- test inputting and outputting SQL standard interval literals
686704
SET IntervalStyle TO sql_standard;
687705
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
@@ -196,6 +196,11 @@ SELECT interval '1 2.3456' minute to second(2);
196196
SELECT interval '1 2:03.5678' minute to second(2);
197197
SELECT interval '1 2:03:04.5678' minute to second(2);
198198

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

0 commit comments

Comments
 (0)