-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sql: do not distribute queries with subqueries returning OIDs #86186
Conversation
11e7c49
to
0a48adb
Compare
pkg/sql/distsql_physical_planner.go
Outdated
@@ -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 t.ResolvedType().Family() == types.OidFamily { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if t is an array or tuple containing an OID?
0a48adb
to
7a8f21e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @cucaroach and @mgartner)
pkg/sql/distsql_physical_planner.go
line 277 at r1 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
What if t is an array or tuple containing an OID?
Good point - it would still be a problem. Fixed.
7a8f21e
to
58b8613
Compare
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
58b8613
to
0643813
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 3 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)
TFTRs! bors r+ |
Build failed (retrying...): |
Build failed (retrying...): |
Build failed (retrying...): |
Build failed (retrying...): |
Build succeeded: |
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.
Fixes: #86075.
Release justification: bug fix.
Release note: None