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.

Sign up for GitHub

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

Jump to bottom

opt: GenerateLookupJoin panics on table with computed columns #59738

Closed
mgartner opened this issue Feb 3, 2021 · 0 comments · Fixed by #59741
Closed

opt: GenerateLookupJoin panics on table with computed columns #59738

mgartner opened this issue Feb 3, 2021 · 0 comments · Fixed by #59741
Assignees
@mgartner
Labels
A-sql-optimizer SQL logical planning and optimizations. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@mgartner
Copy link
Collaborator

mgartner commented Feb 3, 2021 •
edited
Loading

Here's a reproduction:

exec-ddl
CREATE TABLE ab (a INT, b INT AS (a + 10) STORED, INDEX (a))
----

exec-ddl
CREATE TABLE cd (c INT, d INT)
----

opt
SELECT * FROM cd LEFT JOIN ab ON a = c AND d > 5
----

The error:

--- FAIL: TestNormRules (0.00s)
    --- FAIL: TestNormRules/__test (0.00s)
        opt_tester.go:506: testdata/rules/__test:42: runtime error: index out of range [-3]
            (1) assertion failure
            Wraps: (2) attached stack trace
              -- stack trace:
              | github.com/cockroachdb/cockroach/pkg/util/errorutil.ShouldCatch
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/util/errorutil/catch.go:29
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*Optimizer).Optimize.func1
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:205
              | runtime.gopanic
              |         /usr/local/opt/go/libexec/src/runtime/panic.go:969
              | runtime.goPanicIndex
              |         /usr/local/opt/go/libexec/src/runtime/panic.go:88
              | github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/testcat.(*Table).Column
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/testcat/test_catalog.go:656
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*CustomFuncs).findConstantFilterCols
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/select_funcs.go:417
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*CustomFuncs).computedColFilters
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/general_funcs.go:242
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*CustomFuncs).GenerateLookupJoins
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/join_funcs.go:227
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*explorer).exploreLeftJoin
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.og.go:856
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*explorer).exploreGroupMember
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.og.go:26
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*explorer).exploreGroup
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/explorer.go:182
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*Optimizer).optimizeGroup
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:464
              | github.com/cockroachdb/cockroach/pkg/sql/opt/xform.(*Optimizer).Optimize
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/xform/optimizer.go:225
              | github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester.(*OptTester).optimizeExpr
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester/opt_tester.go:1645
              | github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester.(*OptTester).Optimize
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester/opt_tester.go:981
              | github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester.(*OptTester).RunCommand
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/opttester/opt_tester.go:504
              | github.com/cockroachdb/cockroach/pkg/sql/opt/norm_test.TestNormRules.func1.1
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/norm/norm_test.go:45
              | github.com/cockroachdb/datadriven.runDirective.func1
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:321
              | github.com/cockroachdb/datadriven.runDirective
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:326
              | github.com/cockroachdb/datadriven.runDirectiveOrSubTest
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:195
              | github.com/cockroachdb/datadriven.runTestInternal
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:168
              | github.com/cockroachdb/datadriven.RunTest
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:136
              | github.com/cockroachdb/cockroach/pkg/sql/opt/norm_test.TestNormRules.func1
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/pkg/sql/opt/norm/norm_test.go:42
              | github.com/cockroachdb/datadriven.Walk
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:413
              | github.com/cockroachdb/datadriven.Walk.func1
              |         /Users/marcus/go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/datadriven/datadriven.go:426
              | testing.tRunner
              |         /usr/local/opt/go/libexec/src/testing/testing.go:1123
              | runtime.goexit
              |         /usr/local/opt/go/libexec/src/runtime/asm_amd64.s:1374
            Wraps: (3) runtime error: index out of range [-3]
              | -- cause hidden behind barrier
              | runtime error: index out of range [-3]
              | (1) runtime error: index out of range [-3]
              | Error types: (1) runtime.boundsError
            Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierError
@mgartner mgartner added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-optimizer SQL logical planning and optimizations. labels Feb 3, 2021
@mgartner mgartner self-assigned this Feb 3, 2021
mgartner added a commit to mgartner/cockroach that referenced this issue Feb 3, 2021
@mgartner
opt: fix panic in GenerateLookupJoin
69ec085 
In cockroachdb#57690 a new code path was introduced from `findConstantFilterCols`
from `GenerateLookupJoins`. This new code path made it possible for the
filters passed to `findConstantFilterCols` to contain columns that are
not part of the given table. This violated the assumption that the
filter only contains columns in the given table and caused a panic. This
commit fixes the issue by neglecting constant filters for columns not in
the given table.

Fixes cockroachdb#59738

Release note (bug fix): A bug has been fixed that caused errors when
joining two tables when one of the tables had a computed column. This
bug was present since version 21.1.0-alpha.2 and not present in any
production releases.
@mgartner mgartner mentioned this issue Feb 3, 2021
Merged
craig bot pushed a commit that referenced this issue Feb 3, 2021
@aayushshah15 @knz
@mgartner @yuzefovich
Merge #54201 #57170 #59741 #59779
023e4e2 
54201: roachtest: unskip cdc/crdb-chaos r=aayushshah15 a=aayushshah15

I ran this test a total of 15 times in parallel and wasn't able to
reproduce. Since its been skipped for 2+ releases, it's hard to know
what fixed it, but a good guess is #48561.

Release note: None

Fixes #37716 

Informs #36879

Release justification: testing only

57170: util/log: new experimental integration with Fluentd  r=itsbilal a=knz

Release note (cli change): It is now possible to redirect logging to
[Fluentd](https://www.fluentd.org)-compatible network collectors. See
the documentation for details. This is an alpha-quality feature.


59741: opt: fix panic in GenerateLookupJoin r=mgartner a=mgartner

#### opt: fix panic in GenerateLookupJoin

In #57690 a new code path was introduced from `findConstantFilterCols`
from `GenerateLookupJoins`. This new code path made it possible for the
filters passed to `findConstantFilterCols` to contain columns that are
not part of the given table. This violated the assumption that the
filter only contains columns in the given table and caused a panic. This
commit fixes the issue by neglecting constant filters for columns not in
the given table.

Fixes #59738

Release note (bug fix): A bug has been fixed that caused errors when
joining two tables when one of the tables had a computed column. This
bug was present since version 21.1.0-alpha.2 and not present in any
production releases.

#### opt: move findConstantFilterCols to general_funcs.go

Release note: None


59779: flowinfra: deflake a test r=yuzefovich a=yuzefovich

Previously, a unit test could fail in rare circumstances when relocating
a range to a remote node, and now we will use SucceedsSoon to avoid
that. Also unskip the vectorized option.

Fixes: #59712

Release note: None

Co-authored-by: Aayush Shah <aayush.shah15@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@craig craig bot closed this as completed in 6a3e689 Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mgartner mgartner

Labels
A-sql-optimizer SQL logical planning and optimizations. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt: fix panic in GenerateLookupJoin mgartner/cockroach
1 participant
@mgartner