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

distsql: fix EXPLAIN(DISTSQL) ignoring subqueries #25618

Merged
merged 1 commit into from
May 22, 2018
Merged

distsql: fix EXPLAIN(DISTSQL) ignoring subqueries #25618

merged 1 commit into from
May 22, 2018

Conversation

asubiotto
Copy link
Contributor

Fixes #25610

Release note: None

@asubiotto asubiotto requested review from RaduBerinde and a team May 17, 2018 15:35
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RaduBerinde
Copy link
Member

I'd add in the comment or release note how the behavior is changed


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


pkg/sql/explain_distsql.go, line 58 at r1 (raw file):

	}

	auto = ok && auto

The semantics of EXPLAIN (DISTSQL) are that it errors out on unsupported queries. Otherwise the implication is that you can still run them in ON mode.

We can add a flag to prepareForDistSQLSupportCheck for this behavior (and pass p.SessionData().DistSQLMode == sessiondata.DistSQLAlways as the value for the flag in the existing path)


Comments from Reviewable

Fixes #25610

Release note (bug fix): EXPLAIN (DISTSQL) now properly reports that
plans containing subqueries cannot be run through the distributed sql
execution engine.
@asubiotto
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


pkg/sql/explain_distsql.go, line 58 at r1 (raw file):

Previously, RaduBerinde wrote…

The semantics of EXPLAIN (DISTSQL) are that it errors out on unsupported queries. Otherwise the implication is that you can still run them in ON mode.

We can add a flag to prepareForDistSQLSupportCheck for this behavior (and pass p.SessionData().DistSQLMode == sessiondata.DistSQLAlways as the value for the flag in the existing path)

Done.


Comments from Reviewable

@RaduBerinde
Copy link
Member

:lgtm:


Review status: 0 of 4 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@asubiotto
Copy link
Contributor Author

TFTR!
bors r+

craig bot pushed a commit that referenced this pull request May 22, 2018
25618: distsql: fix EXPLAIN(DISTSQL) ignoring subqueries r=asubiotto a=asubiotto

Fixes #25610

Release note: None

Co-authored-by: Alfonso Subiotto Marqués <alfonso@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented May 22, 2018

Build succeeded

@craig craig bot merged commit 87d4189 into cockroachdb:master May 22, 2018
@asubiotto asubiotto deleted the exp-fix branch May 22, 2018 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants