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

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index d7ee865cf750a0fda4ab7c82d43279855bced5af..bdcee15ed1b6692e20fb7fdfa69287467911d947 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -1241,6 +1241,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
@@ -1262,39 +1315,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 c873a99bd94b931930b650dd9c84340d2ee5c0db..946c97ad9249b0d8b7051d7e3da4d8f8a076e0a7 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -682,6 +682,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 789c3de44032c35b26ba67e6029f9efbe91daffe..cff9adab3213cafaaa7056fcd6380a137028b15d 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -196,6 +196,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",