Skip to content

Commit

Permalink
sql: allow refcursor comparison with NULL values
Browse files Browse the repository at this point in the history
Most comparisons are disallowed for the REFCURSOR data type. For example,
the following is not allowed in postgres:
```
SELECT 'foo'::REFCURSOR = 'bar'::REFCURSOR;
```
However, postgres does allow using `IS [NOT] DISTINCT FROM NULL` and
`IS [NOT] NULL` with a refcursor-typed expression os the left operand.
There are a few places where CRDB internally expects this comparison to
be valid as well, so disallowing these comparisons has caused test
failures.

This patch removes the refcursor-specific check for invalid comparisons
during type checking, and relies on execution-time checks as is done for
other comparisons. This means that `IsNull` and `IsNotNull` expressions
can now be executed. In addition, the `IS DISTINCT FROM NULL` comparison
is now supported only for the case of type `REFCURSOR` as the left operand,
and `UNKNOWN` (NULL) as the right operand.

Fixes #112010
Fixes #112011

Release note: None
  • Loading branch information
DrewKimball committed Oct 9, 2023
1 parent 0a99ac2 commit ce698e1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 31 deletions.
70 changes: 59 additions & 11 deletions pkg/sql/logictest/testdata/logic_test/refcursor
Original file line number Diff line number Diff line change
Expand Up @@ -440,29 +440,77 @@ SELECT 'foo'::REFCURSOR >= 'foo'::TEXT;
statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR >= NULL;

# TODO(drewk): Postgres allows this case.
statement error pgcode 42883 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS NULL;

statement error pgcode 42883 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS NOT NULL;

statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS DISTINCT FROM 'foo'::REFCURSOR;

statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS DISTINCT FROM 'foo'::TEXT;

statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS DISTINCT FROM NULL;

statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS NOT DISTINCT FROM 'foo'::REFCURSOR;

statement error pgcode 22023 pq: unsupported comparison operator
SELECT 'foo'::REFCURSOR IS NOT DISTINCT FROM 'foo'::TEXT;

statement error pgcode 22023 pq: unsupported comparison operator
# Regression test for #112010 - REFCURSOR should allow IS NULL and
# IS NOT NULL comparisons.
subtest is_null

query B
SELECT 'foo'::REFCURSOR IS NULL;
----
false

query B
SELECT 'foo'::REFCURSOR IS NOT NULL;
----
true

query B
SELECT 'foo'::REFCURSOR IS DISTINCT FROM NULL;
----
true

query B
SELECT 'foo'::REFCURSOR IS NOT DISTINCT FROM NULL;
----
false

# REFCURSOR columns are allowed.
query B rowsort
SELECT l IS NULL FROM implicit_types;
----
true
true
true
true
true

query B rowsort
SELECT l IS NOT NULL FROM implicit_types;
----
false
false
false
false
false

query B rowsort
SELECT l IS DISTINCT FROM NULL FROM implicit_types;
----
false
false
false
false
false

query B rowsort
SELECT l IS NOT DISTINCT FROM NULL FROM implicit_types;
----
true
true
true
true
true

subtest end
4 changes: 4 additions & 0 deletions pkg/sql/sem/tree/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1775,6 +1775,10 @@ var CmpOps = cmpOpFixups(map[treecmp.ComparisonOperatorSymbol]*CmpOpOverloads{
makeIsFn(types.Void, types.Unknown, volatility.Leakproof),
makeIsFn(types.Unknown, types.Void, volatility.Leakproof),

// REFCURSOR cannot be compared with itself, but as a special case, it can
// be compared with NULL using IS NOT DISTINCT FROM.
makeIsFn(types.RefCursor, types.Unknown, volatility.Leakproof),

// Tuple comparison.
{
LeftType: types.AnyTuple,
Expand Down
20 changes: 0 additions & 20 deletions pkg/sql/sem/tree/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -941,14 +941,6 @@ func (expr *ComparisonExpr) TypeCheck(
return nil, err
}

// REFCURSOR does not support comparisons.
leftType, rightType := leftTyped.ResolvedType(), rightTyped.ResolvedType()
if leftType.Family() == types.RefCursorFamily || rightType.Family() == types.RefCursorFamily {
return nil, pgerror.Newf(pgcode.UndefinedFunction,
"unsupported comparison operator: <%s> %s <%s>", leftType, cmpOp, rightType,
)
}

if alwaysNull {
return DNull, nil
}
Expand Down Expand Up @@ -1471,12 +1463,6 @@ func (expr *IsNullExpr) TypeCheck(
if err != nil {
return nil, err
}
// REFCURSOR does not support comparisons.
if exprTyped.ResolvedType().Family() == types.RefCursorFamily {
return nil, pgerror.New(pgcode.UndefinedFunction,
"unsupported comparison operator: refcursor IS unknown",
)
}
expr.Expr = exprTyped
expr.typ = types.Bool
return expr, nil
Expand All @@ -1490,12 +1476,6 @@ func (expr *IsNotNullExpr) TypeCheck(
if err != nil {
return nil, err
}
// REFCURSOR does not support comparisons.
if exprTyped.ResolvedType().Family() == types.RefCursorFamily {
return nil, pgerror.New(pgcode.UndefinedFunction,
"unsupported comparison operator: refcursor IS NOT unknown",
)
}
expr.Expr = exprTyped
expr.typ = types.Bool
return expr, nil
Expand Down

0 comments on commit ce698e1

Please sign in to comment.