Skip to content

Commit

Permalink
sql: do not distribute queries with subqueries returning OIDs
Browse files Browse the repository at this point in the history
If a subquery results in a DOid datum, the datum will get a type
annotation (because DOids are ambiguous) when serializing the
render expression involving the result of the subquery. As a
result, we might need to perform a cast on a remote node which
might fail, thus we prohibit the distribution of the main query.

Release justification: bug fix.

Release note: None
  • Loading branch information
yuzefovich committed Aug 17, 2022
1 parent 39a86d3 commit 0643813
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 0 deletions.
27 changes: 27 additions & 0 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,16 @@ func (v *distSQLExprCheckVisitor) VisitPre(expr tree.Expr) (recurse bool, newExp
case *tree.DOid:
v.err = newQueryNotSupportedError("OID expressions are not supported by distsql")
return false, expr
case *tree.Subquery:
if hasOidType(t.ResolvedType()) {
// If a subquery results in a DOid datum, the datum will get a type
// annotation (because DOids are ambiguous) when serializing the
// render expression involving the result of the subquery. As a
// result, we might need to perform a cast on a remote node which
// might fail, thus we prohibit the distribution of the main query.
v.err = newQueryNotSupportedError("OID expressions are not supported by distsql")
return false, expr
}
case *tree.CastExpr:
// TODO (rohany): I'm not sure why this CastExpr doesn't have a type
// annotation at this stage of processing...
Expand Down Expand Up @@ -302,6 +312,23 @@ func (v *distSQLExprCheckVisitor) VisitPre(expr tree.Expr) (recurse bool, newExp

func (v *distSQLExprCheckVisitor) VisitPost(expr tree.Expr) tree.Expr { return expr }

// hasOidType returns whether t or its contents include an OID type.
func hasOidType(t *types.T) bool {
switch t.Family() {
case types.OidFamily:
return true
case types.ArrayFamily:
return hasOidType(t.ArrayContents())
case types.TupleFamily:
for _, typ := range t.TupleContents() {
if hasOidType(typ) {
return true
}
}
}
return false
}

// checkExpr verifies that an expression doesn't contain things that are not yet
// supported by distSQL, like distSQL-blocklisted functions.
func checkExpr(expr tree.Expr) error {
Expand Down
21 changes: 21 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/distsql_subquery
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,24 @@ query T
SELECT ARRAY(SELECT a FROM ab ORDER BY b)
----
{1,2,1}

# Regression tests for distributing a query which involves a subquery resulting
# in an OID type (#86075).
statement ok
CREATE TABLE t86075 (k INT PRIMARY KEY, c REGPROCEDURE, a REGPROCEDURE[]);
INSERT INTO t86075 VALUES (1, 1, ARRAY[1]);

statement ok
ALTER TABLE t86075 EXPERIMENTAL_RELOCATE VALUES (ARRAY[2], 1)

statement ok
SELECT (SELECT c FROM t86075) FROM t86075

statement ok
SELECT (SELECT a FROM t86075) FROM t86075

statement ok
SELECT (SELECT (c, c) FROM t86075) FROM t86075

statement ok
SELECT k FROM t86075 WHERE c = (SELECT c FROM t86075 LIMIT 1)
34 changes: 34 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/hash_join_dist
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,37 @@ LEFT JOIN b AS b2 ON (a.id = b2.a_id AND b2.status = 2)
WHERE (a.id IN ('3f90e30a-c87a-4017-b9a0-8f964b91c4af', '3adaf3da-0368-461a-8437-ee448724b78d', 'd0c13b06-5368-4522-8126-105b0a9513cd'))
ORDER BY id DESC
LIMIT 2;

# Some tests with OID types in the equality columns.

statement ok
CREATE TABLE t86075 (k INT PRIMARY KEY, c REGPROCEDURE, a REGPROCEDURE[]);
INSERT INTO t86075 VALUES (1, 1, ARRAY[1]), (2, 2, ARRAY[2]), (3, 3, ARRAY[3]);
CREATE TABLE t86075_2 (k INT PRIMARY KEY, c REGPROCEDURE, a REGPROCEDURE[]);
INSERT INTO t86075_2 VALUES (1, 1, ARRAY[1]), (2, 2, ARRAY[2]), (3, 3, ARRAY[3]);

statement ok
ALTER TABLE t86075 SPLIT AT VALUES (2), (3)

statement ok
ALTER TABLE t86075 SCATTER

statement ok
ALTER TABLE t86075_2 SPLIT AT VALUES (2), (3)

statement ok
ALTER TABLE t86075_2 SCATTER

query I rowsort
SELECT t1.k FROM t86075 AS t1, t86075_2 AS t2 WHERE t1.c = t2.c
----
1
2
3

query I rowsort
SELECT t1.k FROM t86075 AS t1, t86075_2 AS t2 WHERE t1.a = t2.a
----
1
2
3
8 changes: 8 additions & 0 deletions pkg/sql/sem/tree/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,14 @@ type Subquery struct {
typeAnnotation
}

// ResolvedType implements the TypedExpr interface.
func (node *Subquery) ResolvedType() *types.T {
if node.typ == nil {
return types.Any
}
return node.typ
}

// SetType forces the type annotation on the Subquery node.
func (node *Subquery) SetType(t *types.T) {
node.typ = t
Expand Down

0 comments on commit 0643813

Please sign in to comment.