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: bugfix to distsql planning of subqueries #32652

Merged
merged 1 commit into from Nov 27, 2018

Conversation

Projects
None yet
3 participants
@jordanlewis
Copy link
Member

jordanlewis commented Nov 27, 2018

Previously, some subqueries could crash the server when distributed,
because of incorrect accounting for the PlanToStreamColMap final
projection on the result types of the subquery. Now, that final
projection is properly applied.

Fixes #32650.

Release note (bug fix): prevent crash when running certain subqueries
that get planned in a distributed fashion.

@jordanlewis jordanlewis requested a review from RaduBerinde Nov 27, 2018

@jordanlewis jordanlewis requested review from cockroachdb/distsql-prs as code owners Nov 27, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Nov 27, 2018

This change is Reviewable

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

Ha, hold a sec, forgot to finish a comment...

@jordanlewis jordanlewis force-pushed the jordanlewis:fix-subquery-panic branch from f4966ce to 5637ceb Nov 27, 2018

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

OK, PTAL

@RaduBerinde
Copy link
Member

RaduBerinde left a comment

Wow, that was fast. Thanks, LGTM!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/sql/logictest/testdata/logic_test/distsql_subquery, line 26 at r1 (raw file):


statement ok
SELECT ARRAY(SELECT a FROM ab ORDER BY b)

[nit] why not add the result as well (it should always be [1,2,1])

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

Seems like it switches between 1 1 2 and 1 2 1 ... that can't be good

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

Looks like with 5node-dist-opt, the output is out of order.

    --- PASS: TestLogic/5node-dist (0.50s)
        --- PASS: TestLogic/5node-dist/distsql_subquery (0.50s)
    --- FAIL: TestLogic/5node-dist-opt (0.43s)
        --- FAIL: TestLogic/5node-dist-opt/distsql_subquery (0.43s)
            logic.go:2265:

                testdata/logic_test/distsql_subquery:25: SELECT ARRAY(SELECT a FROM ab ORDER BY b)
                expected:
                    {1,2,1}
                but found (query options: "") :
                    {1,1,2}
@RaduBerinde

This comment has been minimized.

Copy link
Member

RaduBerinde commented Nov 27, 2018

Yeah, I had already filed #32648. Just disable the -opt version for now and add a TODO to add it back.

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

Ah, ok, cool!

distsql: bugfix to distsql planning of subqueries
Previously, some subqueries could crash the server when distributed,
because of incorrect accounting for the PlanToStreamColMap final
projection on the result types of the subquery. Now, that final
projection is properly applied.

Release note (bug fix): prevent crash when running certain subqueries
that get planned in a distributed fashion.

@jordanlewis jordanlewis force-pushed the jordanlewis:fix-subquery-panic branch from 5637ceb to b08715d Nov 27, 2018

@jordanlewis

This comment has been minimized.

Copy link
Member

jordanlewis commented Nov 27, 2018

bors r+

craig bot pushed a commit that referenced this pull request Nov 27, 2018

Merge #32652
32652: distsql: bugfix to distsql planning of subqueries r=jordanlewis a=jordanlewis

Previously, some subqueries could crash the server when distributed,
because of incorrect accounting for the PlanToStreamColMap final
projection on the result types of the subquery. Now, that final
projection is properly applied.

Fixes #32650.

Release note (bug fix): prevent crash when running certain subqueries
that get planned in a distributed fashion.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Nov 27, 2018

Build succeeded

@craig craig bot merged commit b08715d into cockroachdb:master Nov 27, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@jordanlewis jordanlewis deleted the jordanlewis:fix-subquery-panic branch Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment