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

sql: internal error: cannot map variable 13 to an indexed var #39435

Closed
maddyblue opened this issue Aug 8, 2019 · 5 comments · Fixed by #39818
Closed

sql: internal error: cannot map variable 13 to an indexed var #39435

maddyblue opened this issue Aug 8, 2019 · 5 comments · Fixed by #39818
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sqlsmith

Comments

@maddyblue
Copy link
Contributor

maddyblue commented Aug 8, 2019

CREATE TABLE IF NOT EXISTS tab_orig AS
	SELECT
		g % 2 = 0 AS _bool, g::STRING AS _bytes
	FROM
		generate_series(0, 0) AS g;

SELECT
	NULL
FROM
	tab_orig AS tab_22780
WHERE
	NOT
		(
			EXISTS(
				SELECT
					(
						SELECT
							NULL
						FROM
							tab_orig
							INNER JOIN tab_orig AS tab_22784 ON tab_22780._bool
						LIMIT
							1
					)
				FROM
					tab_orig AS tab_22781 RIGHT JOIN tab_orig ON true
				WHERE
					true AND '' IN (tab_22781._bytes,)
			)
			AND true
		);
pq: internal error: cannot map variable 13 to an indexed var
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:149: indexedVar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:134: buildVariable()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:91: buildScalar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:163: buildTuple()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:91: buildScalar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:229: buildComparison()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:91: buildScalar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:212: buildBoolean()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:91: buildScalar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:181: buildBoolean()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:91: buildScalar()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:491: buildSelect()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:192: buildRelational()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:1140: buildOrdinality()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:219: buildRelational()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:780: initJoinBuild()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:715: buildHashJoin()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:299: buildRelational()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:528: buildProject()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/relational.go:195: buildRelational()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/builder.go:117: build()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/builder.go:105: Build()
github.com/cockroachdb/cockroach/pkg/sql/apply_join.go:267: Next()
github.com/cockroachdb/cockroach/pkg/sql/plan_node_to_row_source.go:171: Next()
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/base.go:171: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/processors.go:793: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/flow.go:581: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:333: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:942: PlanAndRun()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:910: execWithDistSQLEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:734: dispatchToExecutionEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:417: execStmtInOpenState()
@maddyblue maddyblue added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sqlsmith labels Aug 8, 2019
@jordanlewis
Copy link
Member

Explain(opt):

                                text
+-------------------------------------------------------------------+
  project
   ├── anti-join-apply
   │    ├── scan tab_22780
   │    ├── project
   │    │    └── distinct-on
   │    │         └── right-join (hash)
   │    │              ├── inner-join (hash)
   │    │              │    ├── scan tab_orig
   │    │              │    ├── scan tab_22784
   │    │              │    └── filters (true)
   │    │              ├── ordinality
   │    │              │    └── select
   │    │              │         ├── right-join (hash)
   │    │              │         │    ├── scan tab_22781
   │    │              │         │    ├── scan tab_orig
   │    │              │         │    └── filters (true)
   │    │              │         └── filters
   │    │              │              └── '' IN (tab_22781._bytes,)
   │    │              └── filters
   │    │                   └── variable: tab_22780._bool
   │    └── filters (true)
   └── projections
        └── null
(23 rows)

@jordanlewis
Copy link
Member

@RaduBerinde, would you be able to determine whether this is an optimizer issue or an issue with apply join? Same with #39437.

@RaduBerinde
Copy link
Member

Looks like an optimizer issue, didn't quite get to the bottom of it yet but the problem seems to be that the physical properties in the right side expression are incorrect and have empty presentations (which cause renderNodes to be added to remove columns that we actually need).

    project
     ├── columns:
     ├── stats: [rows=333333.333]
     ├── cost: 1.00000627e+10
     └── distinct-on
          ├── columns:  [hidden: rownum:18]
          ├── grouping columns: rownum:18
          ├── stats: [rows=333333.333, distinct(18)=333333.333, null(18)=0]
          ├── cost: 1.00000593e+10
          ├── key: (18)
          └── right-join (hash)
               ├── columns:  [hidden: tab_22781._bytes:5 rownum:18]
               ├── stats: [rows=3.33333333e+11, distinct(18)=333333.333, null(18)=0]
               ├── cost: 3.33338932e+09
               ├── fd: (18)-->(5)
               ├── inner-join (hash)
               │    ├── columns:
               │    ├── stats: [rows=1000000]
               │    ├── cost: 12150.05
               │    ├── scan tab_orig
               │    │    ├── columns:
               │    │    ├── stats: [rows=1000]
               │    │    └── cost: 1060.02
               │    ├── scan tab_22784
               │    │    ├── columns:
               │    │    ├── stats: [rows=1000]
               │    │    └── cost: 1060.02
               │    └── filters (true)
               ├── ordinality
               │    ├── columns:  [hidden: tab_22781._bytes:5 rownum:18]
               │    ├── stats: [rows=333333.333, distinct(18)=333333.333, null(18)=0]
               │    ├── cost: 25503.4033
               │    ├── key: (18)
               │    ├── fd: (18)-->(5)
               │    └── select
               │         ├── columns:  [hidden: tab_22781._bytes:5]
               │         ├── stats: [rows=333333.333]
               │         ├── cost: 22170.06
               │         ├── right-join (hash)
               │         │    ├── columns:  [hidden: tab_22781._bytes:5]
               │         │    ├── stats: [rows=1000000]
               │         │    ├── cost: 12170.05
               │         │    ├── prune: (5)
               │         │    ├── scan tab_22781
               │         │    │    ├── columns:  [hidden: tab_22781._bytes:5]
               │         │    │    ├── stats: [rows=1000]
               │         │    │    ├── cost: 1080.02
               │         │    │    └── prune: (5)
               │         │    ├── scan tab_orig
               │         │    │    ├── columns:
               │         │    │    ├── stats: [rows=1000]
               │         │    │    └── cost: 1060.02
               │         │    └── filters (true)
               │         └── filters
               │              └── '' IN (tab_22781._bytes,) [outer=(5)]
               └── filters (true)

Note how the columns show up as hidden.

@jordanlewis
Copy link
Member

Thanks @RaduBerinde, very much appreciate the work here.

@RaduBerinde
Copy link
Member

The problem is in the Presentation.Equals method; there is a difference between nil (no presentation) and empty slice (no output columns) but this method doesn't differentiate. This screws with interning. Will have a fix out soon.

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Aug 22, 2019
The nil presentation means that we want all columns in no particular
order, whereas the 0-column presentation means that we don't need any
of the columns (e.g. EXISTS subqueries). Only the nil presentation
is `Any()`. But `Equals()` was incorrectly treating these as equal.
This confused the interner and a nil presentation became a 0-column
presentation which led to some internal errors in Apply.

For completeness, we also fix `HashPhysProps` to differentiate
between these two, but note that this isn't required for correctness.

Fixes cockroachdb#39435.

Release note (bug fix): Fixed internal errors generated during
execution of some complicated cases of correlated subqueries.
craig bot pushed a commit that referenced this issue Aug 23, 2019
39804: opt: add FK benchmarks r=RaduBerinde a=RaduBerinde

Adding simple insert benchmarks comparing no FKs with the old FK path
and the new one.

The results are below. Note that we haven't yet optimized the constant
input case (we can remove the buffer / scan nodes).

Command line:
```
GOMAXPROCS=1 make bench PKG=./pkg/sql/opt/bench BENCHTIMEOUT=30m BENCHES='BenchmarkFKInsert/' TESTFLAGS='-logtostderr NONE -benchtime=20000x -count 5'
```

```
name                                time/op
FKInsert/SingleRow/None              609µs ± 4%
FKInsert/SingleRow/Old               676µs ± 5%
FKInsert/SingleRow/New               961µs ± 1%
FKInsert/MultiRowSingleParent/None  1.37ms ± 1%
FKInsert/MultiRowSingleParent/Old   2.14ms ± 1%
FKInsert/MultiRowSingleParent/New   2.11ms ± 2%
FKInsert/MultiRowMultiParent/None   1.62ms ± 2%
FKInsert/MultiRowMultiParent/Old    2.89ms ± 1%
FKInsert/MultiRowMultiParent/New    2.97ms ± 1%
```

Release note: None

39818: opt: nil presentation should not equal the 0-column presentation r=RaduBerinde a=RaduBerinde

#### sql: enhance apply execbuilder error

We have seen a series of bugs where the execbuilder hits an assertion
error inside Apply. This change adds a detail to the error containing
the formatted opt tree.

Release note: None

#### opt: nil presentation should not equal the 0-column presentation

The nil presentation means that we want all columns in no particular
order, whereas the 0-column presentation means that we don't need any
of the columns (e.g. EXISTS subqueries). Only the nil presentation
is `Any()`. But `Equals()` was incorrectly treating these as equal.
This confused the interner and a nil presentation became a 0-column
presentation which led to some internal errors in Apply.

For completeness, we also fix `HashPhysProps` to differentiate
between these two, but note that this isn't required for correctness.

Fixes #39435.

Release note (bug fix): Fixed internal errors generated during
execution of some complicated cases of correlated subqueries.


Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
@craig craig bot closed this as completed in 9d04015 Aug 23, 2019
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Oct 17, 2019
The nil presentation means that we want all columns in no particular
order, whereas the 0-column presentation means that we don't need any
of the columns (e.g. EXISTS subqueries). Only the nil presentation
is `Any()`. But `Equals()` was incorrectly treating these as equal.
This confused the interner and a nil presentation became a 0-column
presentation which led to some internal errors in Apply.

For completeness, we also fix `HashPhysProps` to differentiate
between these two, but note that this isn't required for correctness.

Fixes cockroachdb#39435.

Release note (bug fix): Fixed internal errors generated during
execution of some complicated cases of correlated subqueries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sqlsmith
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants