summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2016-12-27 15:43:55 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2016-12-27 15:43:55 -0500
commitbeae7d5f0eae0b5ed63151a79b3d5ee853e0925c (patch)
tree654b0fd6275b0b5a03409a3e54f380d047893c55
parent3ba8beda0004742ead13eeeb7d9d17c226746326 (diff)
downloadpostgresql-beae7d5f0eae0b5ed63151a79b3d5ee853e0925c.tar.gz
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
-rw-r--r--src/backend/utils/adt/timestamp.c111
-rw-r--r--src/test/regress/expected/interval.out18
-rw-r--r--src/test/regress/sql/interval.sql5
3 files changed, 105 insertions, 29 deletions
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 3f77656e24..aa710bef29 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -922,6 +922,59 @@ intervaltypmodout(PG_FUNCTION_ARGS)
PG_RETURN_CSTRING(res);
}
+/*
+ * Given an interval typmod value, return a code for the least-significant
+ * field that the typmod allows to be nonzero, for instance given
+ * INTERVAL DAY TO HOUR we want to identify "hour".
+ *
+ * The results should be ordered by field significance, which means
+ * we can't use the dt.h macros YEAR etc, because for some odd reason
+ * they aren't ordered that way. Instead, arbitrarily represent
+ * SECOND = 0, MINUTE = 1, HOUR = 2, DAY = 3, MONTH = 4, YEAR = 5.
+ */
+static int
+intervaltypmodleastfield(int32 typmod)
+{
+ if (typmod < 0)
+ return 0; /* SECOND */
+
+ switch (INTERVAL_RANGE(typmod))
+ {
+ case INTERVAL_MASK(YEAR):
+ return 5; /* YEAR */
+ case INTERVAL_MASK(MONTH):
+ return 4; /* MONTH */
+ case INTERVAL_MASK(DAY):
+ return 3; /* DAY */
+ case INTERVAL_MASK(HOUR):
+ return 2; /* HOUR */
+ case INTERVAL_MASK(MINUTE):
+ return 1; /* MINUTE */
+ case INTERVAL_MASK(SECOND):
+ return 0; /* SECOND */
+ case INTERVAL_MASK(YEAR) | INTERVAL_MASK(MONTH):
+ return 4; /* MONTH */
+ case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR):
+ return 2; /* HOUR */
+ case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
+ return 1; /* MINUTE */
+ case INTERVAL_MASK(DAY) | INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
+ return 0; /* SECOND */
+ case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE):
+ return 1; /* MINUTE */
+ case INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
+ return 0; /* SECOND */
+ case INTERVAL_MASK(MINUTE) | INTERVAL_MASK(SECOND):
+ return 0; /* SECOND */
+ case INTERVAL_FULL_RANGE:
+ return 0; /* SECOND */
+ default:
+ elog(ERROR, "invalid INTERVAL typmod: 0x%x", typmod);
+ break;
+ }
+ return 0; /* can't get here, but keep compiler quiet */
+}
+
/* interval_transform()
* Flatten superfluous calls to interval_scale(). The interval typmod is
@@ -943,39 +996,39 @@ interval_transform(PG_FUNCTION_ARGS)
if (IsA(typmod, Const) &&!((Const *) typmod)->constisnull)
{
Node *source = (Node *) linitial(expr->args);
- int32 old_typmod = exprTypmod(source);
int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue);
- int old_range;
- int old_precis;
- int new_range = INTERVAL_RANGE(new_typmod);
- int new_precis = INTERVAL_PRECISION(new_typmod);
- int new_range_fls;
- int old_range_fls;
-
- if (old_typmod < 0)
- {
- old_range = INTERVAL_FULL_RANGE;
- old_precis = INTERVAL_FULL_PRECISION;
- }
+ bool noop;
+
+ if (new_typmod < 0)
+ noop = true;
else
{
- old_range = INTERVAL_RANGE(old_typmod);
- old_precis = INTERVAL_PRECISION(old_typmod);
- }
+ int32 old_typmod = exprTypmod(source);
+ int old_least_field;
+ int new_least_field;
+ int old_precis;
+ int new_precis;
+
+ old_least_field = intervaltypmodleastfield(old_typmod);
+ new_least_field = intervaltypmodleastfield(new_typmod);
+ if (old_typmod < 0)
+ old_precis = INTERVAL_FULL_PRECISION;
+ else
+ old_precis = INTERVAL_PRECISION(old_typmod);
+ new_precis = INTERVAL_PRECISION(new_typmod);
- /*
- * Temporally-smaller fields occupy higher positions in the range
- * bitmap. Since only the temporally-smallest bit matters for length
- * coercion purposes, we compare the last-set bits in the ranges.
- * Precision, which is to say, sub-second precision, only affects
- * ranges that include SECOND.
- */
- new_range_fls = fls(new_range);
- old_range_fls = fls(old_range);
- if (new_typmod < 0 ||
- ((new_range_fls >= SECOND || new_range_fls >= old_range_fls) &&
- (old_range_fls < SECOND || new_precis >= MAX_INTERVAL_PRECISION ||
- new_precis >= old_precis)))
+ /*
+ * Cast is a no-op if least field stays the same or decreases
+ * while precision stays the same or increases. But precision,
+ * which is to say, sub-second precision, only affects ranges that
+ * include SECOND.
+ */
+ noop = (new_least_field <= old_least_field) &&
+ (old_least_field > 0 /* SECOND */ ||
+ new_precis >= MAX_INTERVAL_PRECISION ||
+ new_precis >= old_precis);
+ }
+ if (noop)
ret = relabel_to_typmod(source, new_typmod);
}
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index a62e85fafe..e90ca2de08 100644
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -692,6 +692,24 @@ SELECT interval '1 2:03:04.5678' minute to second(2);
1 day 02:03:04.57
(1 row)
+-- test casting to restricted precision (bug #14479)
+SELECT f1, f1::INTERVAL DAY TO MINUTE AS "minutes",
+ (f1 + INTERVAL '1 month')::INTERVAL MONTH::INTERVAL YEAR AS "years"
+ FROM interval_tbl;
+ f1 | minutes | years
+-----------------+-----------------+----------
+ 00:01:00 | 00:01:00 | 00:00:00
+ 05:00:00 | 05:00:00 | 00:00:00
+ 10 days | 10 days | 00:00:00
+ 34 years | 34 years | 34 years
+ 3 mons | 3 mons | 00:00:00
+ -00:00:14 | 00:00:00 | 00:00:00
+ 1 day 02:03:04 | 1 day 02:03:00 | 00:00:00
+ 6 years | 6 years | 6 years
+ 5 mons | 5 mons | 00:00:00
+ 5 mons 12:00:00 | 5 mons 12:00:00 | 00:00:00
+(10 rows)
+
-- test inputting and outputting SQL standard interval literals
SET IntervalStyle TO sql_standard;
SELECT interval '0' AS "zero",
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index 78ce9f7bc9..c24612dc9a 100644
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -198,6 +198,11 @@ SELECT interval '1 2.3456' minute to second(2);
SELECT interval '1 2:03.5678' minute to second(2);
SELECT interval '1 2:03:04.5678' minute to second(2);
+-- test casting to restricted precision (bug #14479)
+SELECT f1, f1::INTERVAL DAY TO MINUTE AS "minutes",
+ (f1 + INTERVAL '1 month')::INTERVAL MONTH::INTERVAL YEAR AS "years"
+ FROM interval_tbl;
+
-- test inputting and outputting SQL standard interval literals
SET IntervalStyle TO sql_standard;
SELECT interval '0' AS "zero",