From dde16a878a25813a6413e9e417e286a65f8471ce Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Mon, 6 Jun 2022 16:54:40 -0400
Subject: [PATCH] eval: show error when comparing OID to int out of range

Release note (sql change): An error is now returned if an OID is
compared to an integer that is out of the allowable range of OID input
values, which is [MinInt32, MaxUint32].
---
 .../logictest/testdata/logic_test/pgoidtype   | 37 ++++++++++++++++
 pkg/sql/sem/eval/binary_op.go                 |  5 ++-
 pkg/sql/sem/eval/testdata/eval/oid            | 44 +++++++++++++++++++
 pkg/sql/sem/tree/datum.go                     |  6 +++
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/pgoidtype b/pkg/sql/logictest/testdata/logic_test/pgoidtype
index fe1a615d3e24..695763392b48 100644
--- a/pkg/sql/logictest/testdata/logic_test/pgoidtype
+++ b/pkg/sql/logictest/testdata/logic_test/pgoidtype
@@ -525,3 +525,40 @@ regression_62205
 # Check we error as appropriate if the OID type is not legit.
 statement error pgcode 22P02 invalid input syntax for type oid: "regression_69907"
 SELECT 'regression_69907'::oid
+
+# 4294967295 is MaxUint32.
+# -2147483648 is MinInt32.
+query OIBB
+SELECT o, i, o > i, i > o FROM (VALUES
+  (1::oid, 4294967295::int8),
+  (1::oid, -2147483648::int8),
+  ((-1)::oid, 4294967295::int8),
+  ((-1)::oid, -2147483648::int8),
+  ((-2147483648)::oid, 4294967295::int8),
+  ((-2147483648)::oid, -2147483648::int8),
+  (4294967295::oid, 4294967295::int8),
+  (4294967295::oid, -2147483648::int8)
+) tbl(o, i)
+----
+1           4294967295   false  true
+1           -2147483648  false  true
+4294967295  4294967295   false  false
+4294967295  -2147483648  true   false
+2147483648  4294967295   false  true
+2147483648  -2147483648  false  false
+4294967295  4294967295   false  false
+4294967295  -2147483648  true   false
+
+# 4294967296 is (MaxUint32 + 1).
+query error OID out of range: 4294967296
+SELECT 1:::OID >= 4294967296:::INT8
+
+query error OID out of range: 4294967296
+SELECT 4294967296:::INT8 >= 1:::OID
+
+# -2147483649 is (MinInt32 - 1).
+query error OID out of range: -2147483649
+SELECT 1:::OID >= -2147483649:::INT8
+
+query error OID out of range: -2147483649
+SELECT -2147483649:::INT8 >= 1:::OID
diff --git a/pkg/sql/sem/eval/binary_op.go b/pkg/sql/sem/eval/binary_op.go
index 7b351578fabd..3f9efed32b27 100644
--- a/pkg/sql/sem/eval/binary_op.go
+++ b/pkg/sql/sem/eval/binary_op.go
@@ -140,7 +140,10 @@ func (e *evaluator) EvalCompareScalarOp(
 			return tree.DNull, nil
 		}
 	}
-	cmp := left.Compare(e.ctx(), right)
+	cmp, err := left.CompareError(e.ctx(), right)
+	if err != nil {
+		return nil, err
+	}
 	return boolFromCmp(cmp, op.ComparisonOperator), nil
 }
 
diff --git a/pkg/sql/sem/eval/testdata/eval/oid b/pkg/sql/sem/eval/testdata/eval/oid
index 8fed6b1bf7b0..bcc755002cf9 100644
--- a/pkg/sql/sem/eval/testdata/eval/oid
+++ b/pkg/sql/sem/eval/testdata/eval/oid
@@ -82,3 +82,47 @@ eval
 1:::OID <= -3:::INT
 ----
 true
+
+# 4294967295 is MaxUint32.
+eval
+1:::OID >= 4294967295:::INT8
+----
+false
+
+eval
+4294967295:::INT8 >= 1:::OID
+----
+true
+
+# 4294967296 is (MaxUint32 + 1).
+eval
+1:::OID >= 4294967296:::INT8
+----
+OID out of range: 4294967296
+
+eval
+4294967296:::INT8 >= 1:::OID
+----
+OID out of range: 4294967296
+
+# -2147483648 is MinInt32.
+eval
+1:::OID >= -2147483648:::INT8
+----
+false
+
+eval
+-2147483648:::INT8 >= 1:::OID
+----
+true
+
+# -2147483649 is (MinInt32 - 1).
+eval
+1:::OID >= -2147483649:::INT8
+----
+OID out of range: -2147483649
+
+eval
+-2147483649:::INT8 >= 1:::OID
+----
+OID out of range: -2147483649
diff --git a/pkg/sql/sem/tree/datum.go b/pkg/sql/sem/tree/datum.go
index b8838f3c1dd0..67cb8453e3ed 100644
--- a/pkg/sql/sem/tree/datum.go
+++ b/pkg/sql/sem/tree/datum.go
@@ -759,6 +759,9 @@ func (d *DInt) CompareError(ctx CompareContext, other Datum) (int, error) {
 		// OIDs are always unsigned 32-bit integers. Some languages, like Java,
 		// compare OIDs to signed 32-bit integers, so we implement the comparison
 		// by converting to a uint32 first. This matches Postgres behavior.
+		if thisInt > math.MaxUint32 || thisInt < math.MinInt32 {
+			return 0, pgerror.Newf(pgcode.NumericValueOutOfRange, "OID out of range: %d", thisInt)
+		}
 		thisInt = DInt(uint32(thisInt))
 		v = DInt(t.Oid)
 	default:
@@ -4963,6 +4966,9 @@ func (d *DOid) CompareError(ctx CompareContext, other Datum) (int, error) {
 		// OIDs are always unsigned 32-bit integers. Some languages, like Java,
 		// compare OIDs to signed 32-bit integers, so we implement the comparison
 		// by converting to a uint32 first. This matches Postgres behavior.
+		if *t > math.MaxUint32 || *t < math.MinInt32 {
+			return 0, pgerror.Newf(pgcode.NumericValueOutOfRange, "OID out of range: %d", *t)
+		}
 		v = oid.Oid(*t)
 	default:
 		return 0, makeUnsupportedComparisonMessage(d, other)