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: invalid index #39433

Closed
maddyblue opened this issue Aug 7, 2019 · 6 comments · Fixed by #66442
Closed

sql: internal error: invalid index #39433

maddyblue opened this issue Aug 7, 2019 · 6 comments · Fixed by #66442
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. O-sqlsmith

Comments

@maddyblue
Copy link
Contributor

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

SELECT
	(
		SELECT
			NULL
		FROM
			tab_orig
			LEFT JOIN tab_orig AS tab_57077
				RIGHT JOIN tab_orig AS tab_57078
					FULL JOIN tab_orig AS tab_57079 ON
							true ON tab_57069._bool
				CROSS JOIN tab_orig AS tab_57080
				INNER JOIN tab_orig AS tab_57081 ON true ON
					EXISTS(
						SELECT
							NULL
						FROM
							tab_orig AS tab_57082
							LEFT JOIN tab_orig ON
									EXISTS(
										SELECT
											NULL
										FROM
											tab_orig
											
									)
					)
	)
FROM
	tab_orig AS tab_57069;
pq: internal error: invalid index 3 for "EXISTS (SELECT NULL FROM tab_orig)"
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/sql/subquery.go:46: EvalSubquery()
github.com/cockroachdb/cockroach/pkg/sql/distsqlplan/expression.go:119: VisitPre()
github.com/cockroachdb/cockroach/pkg/sql/sem/tree/walk.go:667: WalkExpr()
github.com/cockroachdb/cockroach/pkg/sql/distsqlplan/expression.go:86: MakeExpression()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:894: initTableReaderSpec()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:1070: createTableReaders()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:2312: createPlanForNode()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:2197: createPlanForJoin()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:2324: createPlanForNode()
github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:2365: createPlanForNode()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:826: planAndRunSubquery()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:764: PlanAndRunSubqueries()
github.com/cockroachdb/cockroach/pkg/sql/apply_join.go:303: runRightSidePlan()
github.com/cockroachdb/cockroach/pkg/sql/apply_join.go:273: 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()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:99: execStmt()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1211: execCmd()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1140: run()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:442: ServeConn()
github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:584: func1()
@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 7, 2019
@jordanlewis
Copy link
Member

This issue is caused by a weird combination of subqueries. It seems like one of the subqueries has an embedded subquery, and when planning the outer subquery, the inner subquery isn't yet available, or something?

Here's the explain plan:

            tree           |    field     |                                description
+--------------------------+--------------+----------------------------------------------------------------------------+
  root                     |              |
   ├── render              |              |
   │    └── apply-join     |              |
   │         │             | type         | left outer
   │         └── scan      |              |
   │                       | table        | tab_orig@primary
   │                       | spans        | ALL
   └── subquery            |              |
        │                  | id           | @S1
        │                  | original sql | EXISTS (SELECT NULL FROM tab_orig AS tab_57082 LEFT JOIN tab_orig ON true)
        │                  | exec mode    | exists
        └── limit          |              |
             │             | count        | 1
             └── hash-join |              |
                  │        | type         | left outer
                  ├── scan |              |
                  │        | table        | tab_orig@primary
                  │        | spans        | ALL
                  └── scan |              |
                           | table        | tab_orig@primary
                           | spans        | ALL
(21 rows)

Explain opt:

                                   text
+------------------------------------------------------------------------+
  project
   ├── left-join-apply
   │    ├── scan tab_57069
   │    ├── max1-row
   │    │    └── project
   │    │         ├── left-join (hash)
   │    │         │    ├── scan tab_orig
   │    │         │    ├── inner-join (hash)
   │    │         │    │    ├── inner-join (hash)
   │    │         │    │    │    ├── right-join (hash)
   │    │         │    │    │    │    ├── scan tab_57077
   │    │         │    │    │    │    ├── full-join (hash)
   │    │         │    │    │    │    │    ├── scan tab_57078
   │    │         │    │    │    │    │    ├── scan tab_57079
   │    │         │    │    │    │    │    └── filters (true)
   │    │         │    │    │    │    └── filters
   │    │         │    │    │    │         └── variable: tab_57069._bool
   │    │         │    │    │    ├── scan tab_57080
   │    │         │    │    │    └── filters (true)
   │    │         │    │    ├── scan tab_57081
   │    │         │    │    └── filters (true)
   │    │         │    └── filters
   │    │         │         └── exists
   │    │         │              └── limit
   │    │         │                   ├── left-join (hash)
   │    │         │                   │    ├── scan tab_57082
   │    │         │                   │    ├── scan tab_orig
   │    │         │                   │    └── filters (true)
   │    │         │                   └── const: 1
   │    │         └── projections
   │    │              └── null
   │    └── filters (true)
   └── projections
        └── variable: ?column?
(34 rows)

@jordanlewis
Copy link
Member

I think the issue here is perhaps that a subquery within an apply join expects to be able to reference a subquery outside of the apply join, but it cannot because the subqueries array passed to the apply join is solely the subqueries that are contained within the apply join itself??

I don't know, I'm lost. @RaduBerinde can you take a look? Sorry for the deluge of apply join stuff that we can't figure out.

@RaduBerinde
Copy link
Member

I see.. when we construct a new plan with the execbuilder, we'd want to include the subqueries of the parent plan (but not execute those with PlanAndRunSubqueries). It's simple conceptually but not easy to make it work I think. It might help to make tree.Subquery refer to something less fragile than an index in a slice. We may also want to have a factory function to "allocate" a subquery when we create tree.Subquery, rather than passing the subquery info in a slice at the end. Some more thought is needed.

While it's important to iron out these issues, I don't think this work is a high priority unless a user runs into problems (it's a corner case of apply, which itself is a corner case). If it's impeding other sqlsmith coverage, we could add a knob for disabling apply joins and do runs with them off.

@asubiotto
Copy link
Contributor

@RaduBerinde it seems like the actionable item here is around plan creation so moving to triage in SQL Planning.

@yuzefovich
Copy link
Member

We hit another several instances of this bug, all on sqlsmith queries (#54166), here is a reduced one:

SELECT
  (
    SELECT
      tab_4.col_4
    FROM
      (VALUES (1)) AS tab_1 (col_1)
      JOIN (
          VALUES
            (
              (
                SELECT
                  1
                FROM
                  (SELECT 1)
                WHERE
                  EXISTS(SELECT 1)
              )
            )
        )
          AS tab_6 (col_6) ON (tab_1.col_1) = (tab_6.col_6)
  )
FROM
  (VALUES (NULL)) AS tab_4 (col_4),
  (VALUES (NULL), (NULL)) AS tab_5 (col_5);

@yuzefovich
Copy link
Member

Another reproduction is here.

yuzefovich added a commit to yuzefovich/cockroach that referenced this issue Dec 28, 2020
This commit temporarily skips failing `sqlsmith` roachtest on `internal
error: invalid index` errors which are due to issues with subqueries
(cockroachdb#39433) in order to reduce the noise.

Release note: None
craig bot pushed a commit that referenced this issue Dec 28, 2020
58249: roachtest: improve sqlsmith r=yuzefovich a=yuzefovich

**roachtest: delay panic injection in sqlsmith**

We need to delay the panic injection until after the smither is created
because some queries are run during its instantiation.

Fixes: #58235.

Release note: None

**roachtest: ignore a known issue in sqlsmith**

This commit temporarily skips failing `sqlsmith` roachtest on `internal
error: invalid index` errors which are due to issues with subqueries
(#39433) in order to reduce the noise.

Release note: None

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
yuzefovich added a commit to yuzefovich/cockroach that referenced this issue Mar 22, 2021
This commit temporarily skips failing `sqlsmith` roachtest on `internal
error: invalid index` errors which are due to issues with subqueries
(cockroachdb#39433) in order to reduce the noise.

Release note: None
@rytaft rytaft added the E-quick-win Likely to be a quick win for someone experienced. label May 10, 2021
craig bot pushed a commit that referenced this issue May 10, 2021
64663: sql: fix error on ALTER/DROP TYPE on builtin type r=postamar a=postamar

Previously, an assertion failure would be raised when executing
a statement along the lines of:

    ALTER TYPE foo.GEOMETRY RENAME TO bar

The name GEOMETRY is one of a handful of builtin types that warrant
special treatment during name resolution. This commit instead causes
a pgerror to be raised when attempting to mutate a builtin type.

Fixes #64398

Release note: None

64767: sql: make error during subquery eval more specific r=yuzefovich a=yuzefovich

We have a known limitation (#39433) around subqueries and apply joins, so
we ignore errors that occur because of it during `sqlsmith` roachtest.
This commit makes the error message a bit more specific so that we
didn't mistakenly ignore other issues.

Release note: None

Co-authored-by: Marius Posta <marius@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@craig craig bot closed this as completed in da3ff89 Jun 14, 2021
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. E-quick-win Likely to be a quick win for someone experienced. O-sqlsmith
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants