Skip to content
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

opt: desired scalar types should propagate into subqueries #37263

Closed
jordanlewis opened this issue May 2, 2019 · 0 comments · Fixed by #37578
Closed

opt: desired scalar types should propagate into subqueries #37263

jordanlewis opened this issue May 2, 2019 · 0 comments · Fixed by #37578
Assignees
Labels
A-sql-typing SQLtype inference, typing rules, type compatibility.

Comments

@jordanlewis
Copy link
Member

Currently, types in comparisons and other contexts don't propagate properly if the RHS of the comparison is a subquery. For example, this query should work because the right hand numeric literal should be able to notice that it needs to be a decimal:

root@127.0.0.1:51930/defaultdb> select 3::decimal in (select 1);
pq: unsupported comparison operator: <decimal> IN <tuple{int}>

The root cause of this problem is that subqueries are built without the type context of their surroundings in the optimizer (and HP too).

@rytaft and I discussed the problem for a bit and think that the missing link is propagating the desired scalar types into replaceSubquery, which is the method that ultimately calls buildStmt and begins the optbuilder recursion for a subquery. buildStmt passes nil for desired types into buildSelect, which needs to instead be the desired scalar types.

The problem is that replaceSubquery happens too early, before types of the surrounding expressions are available. One solution might be to delay calling buildStmt until later, when the types are available, in the buildMultiRowSubquery and buildSingleRowSubquery methods.

Fixing this would resolve #14554 and a couple of other similar issues, so it would be great to get done if not too tricky.

@jordanlewis jordanlewis added this to To do in BACKLOG, NO NEW ISSUES: SQL Optimizer via automation May 2, 2019
@jordanlewis jordanlewis added the A-sql-typing SQLtype inference, typing rules, type compatibility. label May 2, 2019
rytaft added a commit to rytaft/cockroach that referenced this issue May 17, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was AnyTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
rytaft added a commit to rytaft/cockroach that referenced this issue May 20, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was AnyTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
craig bot pushed a commit that referenced this issue May 20, 2019
37506: storage: reconcile manual splitting with automatic merging r=jeffrey-xiao a=jeffrey-xiao

Follows the steps outlined in #37487 to reconcile manual splitting with automatic merging. This PR includes the changes needed at the storage layer. The sticky bit indicating that a range is manually split is added to the range descriptor.

37558: docs/tla-plus: add timestamp refreshes to ParallelCommits spec r=nvanbenschoten a=nvanbenschoten

This commit adds transaction timestamp refreshes to the PlusCal model
for parallel commits. This allows the committing transaction to bump
its timestamp without a required epoch bump.

This completes the parallel commits formal specification.

37578: opt, sql: fix type inference of TypeCheck for subqueries r=rytaft a=rytaft

Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until `TypeCheck` is called for
   the first time.

2. For subqueries on the right side of an `IN` expression, the desired type
   passed into `TypeCheck` was `AnyTuple`. This resulted in an error later on in
   `typeCheckSubqueryWithIn`, which checks to make sure the type of the subquery
   is `tuple{T}` where `T` is the type of the left expression. Now the desired
   type passed into `TypeCheck` is `tuple{T}`.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes #37263
Fixes #14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.

Co-authored-by: Jeffrey Xiao <jeffrey.xiao1998@gmail.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
@craig craig bot closed this as completed in #37578 May 20, 2019
BACKLOG, NO NEW ISSUES: SQL Optimizer automation moved this from To do to Done May 20, 2019
rytaft added a commit to rytaft/cockroach that referenced this issue May 20, 2019
Prior to this commit, the optimizer was not correctly inferring the types of
columns in subqueries for expressions of the form `scalar IN (subquery)`.
This was due to two problems which have now been fixed:

1. The subquery was built as a relational expression before the desired types
   were known. Now the subquery build is delayed until TypeCheck is called for
   the first time.

2. For subqueries on the right side of an IN expression, the desired type
   passed into TypeCheck was FamTuple. This resulted in an error later on in
   typeCheckSubqueryWithIn, which checks to make sure the type of the subquery
   is tuple{T} where T is the type of the left expression. Now the desired
   type passed into TypeCheck is tuple{T}.

Note that this commit only fixes type inference for the optimizer. It is still
broken in the heuristic planner.

Fixes cockroachdb#37263
Fixes cockroachdb#14554

Release note (bug fix): Fixed type inference of columns in subqueries for
some expressions of the form `scalar IN (subquery)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-typing SQLtype inference, typing rules, type compatibility.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants