sql/opt: prevent NormalizeCmpTimeZoneFunction from masking bounds errors

pkg/sql/logictest/testdata/logic_test/timestamp

@@ -829,3 +829,36 @@ SELECT '-infinity'::timestamp + '1 second'::interval, '-infinity'::timestamp - '
 -infinity -infinity
 
 subtest end
+
+# Regression test for #169202. The NormalizeCmpTimeZoneFunction rule must not
+# rewrite timezone(zone, ts) op const into ts op timezone(zone, const) when
+# the rewrite could hide a runtime "exceeds supported timestamp bounds" error
+# that the original expression would have produced for boundary input rows.
+subtest regression_normalize_cmp_timezone_bounds
+
+statement ok
+CREATE TABLE timezone_bounds_t (ts TIMESTAMP)
+
+statement ok
+INSERT INTO timezone_bounds_t VALUES ('294276-12-31 23:59:59.999999')
+
+# Sanity check: with the rule disabled, per-row evaluation errors as
+# expected on the boundary row.
+statement ok
+SET disable_optimizer_rules = 'NormalizeCmpTimeZoneFunction'
+
+query error exceeds supported timestamp bounds
+SELECT timezone('America/Denver', ts) >= '2020-06-01 12:35:55-07' FROM timezone_bounds_t
+
+statement ok
+SET disable_optimizer_rules = ''
+
+# With the rule enabled (default), the rewrite must not silently mask
+# the bounds error.
+query error exceeds supported timestamp bounds
+SELECT timezone('America/Denver', ts) >= '2020-06-01 12:35:55-07' FROM timezone_bounds_t
+
+statement ok
+DROP TABLE timezone_bounds_t
+
+subtest end

pkg/sql/opt/norm/comp_funcs.go

@@ -6,12 +6,15 @@
 package norm
 
 import (
+	"time"
+
 	"github.com/cockroachdb/cockroach/pkg/sql/opt"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/builtinsregistry"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 )
 
@@ -77,6 +80,61 @@ func (c *CustomFuncs) NormalizeTupleEquality(left, right memo.ScalarListExpr) op
 	return result
 }
 
+// CanTimeZoneFunctionOverflow returns true if the NormalizeCmpTimeZoneFunction
+// or NormalizeCmpTimeZoneFunctionTZ rewrite could mask a runtime
+// "exceeds supported timestamp bounds" error. The rewrite hoists a per-row
+// timezone(zone, ts) call out of a comparison; if any value in the column
+// domain would push the timezone-shifted result past the supported timestamp
+// bounds, the original expression would have errored at evaluation while the
+// rewritten predicate would not. The rewrite is only safe when applying the
+// timezone shift to both MinSupportedTime and MaxSupportedTime succeeds, which
+// is a sufficient condition that no value in the column can trigger the
+// bounds error.
+//
+// Returns true (rewrite is unsafe) if zone is not a constant string, or if
+// applying the timezone shift to either supported-time bound errors.
+func (c *CustomFuncs) CanTimeZoneFunctionOverflow(zone, ts opt.ScalarExpr) bool {
+	zoneConst, ok := zone.(*memo.ConstExpr)
+	if !ok {
+		return true
+	}
+	zoneStr, ok := zoneConst.Value.(*tree.DString)
+	if !ok {
+		return true
+	}
+	loc, err := timeutil.TimeZoneStringToLocation(
+		string(*zoneStr), timeutil.TimeZoneStringToLocationPOSIXStandard,
+	)
+	if err != nil {
+		return true
+	}
+	// The rule normalizes timezone(zone, ts) where ts is of type TIMESTAMP or
+	// TIMESTAMPTZ. The timezone() overload chosen by MakeTimeZoneFunction
+	// determines whether the shift is AddTimeZone (TIMESTAMP -> TIMESTAMPTZ)
+	// or EvalAtAndRemoveTimeZone (TIMESTAMPTZ -> TIMESTAMP). Either direction
+	// is an additive shift that can push a boundary value out of bounds.
+	minTS := &tree.DTimestamp{Time: tree.MinSupportedTime}
+	maxTS := &tree.DTimestamp{Time: tree.MaxSupportedTime}
+	if ts.DataType().Family() == types.TimestampFamily {
+		if _, err := minTS.AddTimeZone(loc, time.Microsecond); err != nil {
+			return true
+		}
+		if _, err := maxTS.AddTimeZone(loc, time.Microsecond); err != nil {
+			return true
+		}
+		return false
+	}
+	minTZ := &tree.DTimestampTZ{Time: tree.MinSupportedTime}
+	maxTZ := &tree.DTimestampTZ{Time: tree.MaxSupportedTime}
+	if _, err := minTZ.EvalAtAndRemoveTimeZone(loc, time.Microsecond); err != nil {
+		return true
+	}
+	if _, err := maxTZ.EvalAtAndRemoveTimeZone(loc, time.Microsecond); err != nil {
+		return true
+	}
+	return false
+}
+
 // MakeTimeZoneFunction constructs a new timezone() function with the given zone
 // and timestamp as arguments. The type of the function result is TIMESTAMPTZ if
 // ts is of type TIMESTAMP, or TIMESTAMP if is of type TIMESTAMPTZ.

pkg/sql/opt/norm/rules/comp.opt

@@ -206,6 +206,10 @@
 #   1. The left side of the comparison is a timezone() function.
 #   2. The second argument to timezone() is a variable of type TIMESTAMP.
 #   3. The right side of the comparison is a constant value TIMESTAMPTZ.
+#   4. Applying the zone shift to the supported timestamp bounds does not
+#      overflow. This ensures the rewrite cannot mask a per-row
+#      "exceeds supported timestamp bounds" error that the original
+#      expression would have produced (see #169202).
 #
 # Here's an example:
 #
@@ -221,7 +225,8 @@
     $right:(ConstValue) &
         (IsTimestampTZ $right) &
         (Let ($zone $ts $ok):(ScalarPair $args) $ok) &
-        (IsTimestamp $ts)
+        (IsTimestamp $ts) &
+        ^(CanTimeZoneFunctionOverflow $zone $ts)
 )
 =>
 ((OpName) $ts (MakeTimeZoneFunction $zone $right))
@@ -232,6 +237,10 @@
 #   1. The left side of the comparison is a timezone() function.
 #   2. The second argument to timezone() is a variable of type TIMESTAMPTZ.
 #   3. The right side of the comparison is a constant value TIMESTAMP.
+#   4. Applying the zone shift to the supported timestamp bounds does not
+#      overflow. This ensures the rewrite cannot mask a per-row
+#      "exceeds supported timestamp bounds" error that the original
+#      expression would have produced (see #169202).
 #
 # Here's an example:
 #
@@ -248,7 +257,8 @@
         (IsTimestamp $right) &
         (Let ($zone $tz $ok):(ScalarPair $args) $ok) &
         (IsTimestampTZ $tz) &
-        ^(IsConstValueOrGroupOfConstValues $tz)
+        ^(IsConstValueOrGroupOfConstValues $tz) &
+        ^(CanTimeZoneFunctionOverflow $zone $tz)
 )
 =>
 ((OpName) $tz (MakeTimeZoneFunction $zone $right))