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

sem/tree: v21.2.7: incorrect type conversion in ColumnAccessExpr - maybe DNull check is missing? #78159

Closed
cockroach-teamcity opened this issue Mar 20, 2022 · 2 comments · Fixed by #78423
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. T-sql-queries SQL Queries Team

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Mar 20, 2022

This issue was autofiled by Sentry. It represents a crash or reported error on a live cluster with telemetry enabled.

Sentry link: https://sentry.io/organizations/cockroach-labs/issues/3118425943/?referrer=webhooks_plugin

Panic message:

error.go:88: unexpected error from the vectorized engine: ×
--
*barriers.barrierError
*errutil.withPrefix: unexpected error from the vectorized engine (1)
error.go:88: *withstack.withStack (top exception)
*assert.withAssertionFailure
*colexecerror.notInternalError
*colexecerror.notInternalError
(check the extra data payloads)

Stacktrace (expand for inline code snippets):

// unexpected.
retErr = errors.NewAssertionErrorWithWrappedErrf(err, "unexpected error from the vectorized engine")
}
in pkg/sql/colexecerror.CatchVectorizedRuntimeError.func1
/usr/local/go/src/runtime/panic.go#L964-L966 in runtime.gopanic
/usr/local/go/src/runtime/iface.go#L260-L262 in runtime.panicdottypeE
/usr/local/go/src/runtime/iface.go#L270-L272 in runtime.panicdottypeI
}
return d.(*DTuple).D[expr.ColIndex], nil
}
in pkg/sql/sem/tree.(*ColumnAccessExpr).Eval
func (expr *IsNotNullExpr) Eval(ctx *EvalContext) (Datum, error) {
d, err := expr.Expr.(TypedExpr).Eval(ctx)
if err != nil {
in pkg/sql/sem/tree.(*IsNotNullExpr).Eval
d, err := filter.Eval(evalCtx)
if err != nil {
in pkg/sql/execinfrapb.RunFilter
eh.evalCtx.PushIVarContainer(eh)
pass, err := RunFilter(eh.Expr, eh.evalCtx)
eh.evalCtx.PopIVarContainer()
in pkg/sql/execinfrapb.(*ExprHelper).EvalFilter
// Perform the actual filtering.
passes, err := f.filter.EvalFilter(row)
if err != nil {
in pkg/sql/rowexec.(*filtererProcessor).Next
var p *execinfrapb.ProducerMetadata
r.row, p = r.source.Next()
in pkg/sql.(*rowSourceToPlanNode).Next
// We need a new row on the left.
ok, err := a.input.plan.Next(params)
if err != nil {
in pkg/sql.(*applyJoinNode).Next
for p.State == execinfra.StateRunning {
valid, err := p.node.Next(p.params)
if err != nil || !valid {
in pkg/sql.(*planNodeToRowSource).Next
for ; nRows < c.batch.Capacity(); nRows++ {
row, meta := c.input.Next()
if meta != nil {
in pkg/sql/colexec.(*Columnarizer).Next
// Get a fresh batch.
m.batch = m.input.Next()
if m.batch.Length() == 0 {
in pkg/sql/colexec.(*Materializer).next
func (m *Materializer) nextAdapter() {
m.outputRow = m.next()
}
in pkg/sql/colexec.(*Materializer).nextAdapter
}()
operation()
return retErr
in pkg/sql/colexecerror.CatchVectorizedRuntimeError
for m.State == execinfra.StateRunning {
if err := colexecerror.CatchVectorizedRuntimeError(m.nextAdapter); err != nil {
m.MoveToDraining(err)
in pkg/sql/colexec.(*Materializer).Next
if f.State == execinfra.StateRunning {
row, meta := f.input.Next()
if meta != nil {
in pkg/sql/colflow.(*FlowCoordinator).next
func (f *FlowCoordinator) nextAdapter() {
f.row, f.meta = f.next()
}
in pkg/sql/colflow.(*FlowCoordinator).nextAdapter
}()
operation()
return retErr
in pkg/sql/colexecerror.CatchVectorizedRuntimeError
func (f *FlowCoordinator) Next() (rowenc.EncDatumRow, *execinfrapb.ProducerMetadata) {
if err := colexecerror.CatchVectorizedRuntimeError(f.nextAdapter); err != nil {
f.MoveToDraining(err)
in pkg/sql/colflow.(*FlowCoordinator).Next
for {
row, meta := src.Next()
// Emit the row; stop if no more rows are needed.
in pkg/sql/execinfra.Run
pb.self.Start(ctx)
Run(pb.Ctx, pb.self, pb.Output)
}
in pkg/sql/execinfra.(*ProcessorBaseNoHelper).Run
log.VEventf(ctx, 1, "running %T in the flow's goroutine", headProc)
headProc.Run(ctx)
}
in pkg/sql/flowinfra.(*FlowBase).Run
// as the root, so we run this flow with the default implementation.
f.FlowBase.Run(ctx, doneFn)
return
in pkg/sql/colflow.(*vectorizedFlow).Run
// TODO(radu): this should go through the flow scheduler.
flow.Run(ctx, func() {})
in pkg/sql.(*DistSQLPlanner).Run
recv.expectedRowsRead = int64(physPlan.TotalEstimatedScannedRows)
runCleanup := dsp.Run(planCtx, txn, physPlan, recv, evalCtx, nil /* finishedSetupFn */)
return func() {
in pkg/sql.(*DistSQLPlanner).PlanAndRun
params.p.extendedEvalCtx.ExecCfg.DistSQLPlanner.PlanAndRun(
params.ctx, evalCtx, planCtx, params.p.Txn(), plan.main, recv,
in pkg/sql.runPlanInsidePlan
if err := runPlanInsidePlan(params, newPlan.(*planComponents), &n.workingRows); err != nil {
return false, err
in pkg/sql.(*recursiveCTENode).Next
for p.State == execinfra.StateRunning {
valid, err := p.node.Next(p.params)
if err != nil || !valid {
in pkg/sql.(*planNodeToRowSource).Next
for ; nRows < c.batch.Capacity(); nRows++ {
row, meta := c.input.Next()
if meta != nil {
in pkg/sql/colexec.(*Columnarizer).Next
bic.stopwatch.Start()
batch = bic.Operator.Next()
bic.stopwatch.Stop()
in pkg/sql/colflow.(*batchInfoCollector).Next

pkg/sql/colexecerror/error.go in pkg/sql/colexecerror.CatchVectorizedRuntimeError.func1 at line 88
/usr/local/go/src/runtime/panic.go in runtime.gopanic at line 965
/usr/local/go/src/runtime/iface.go in runtime.panicdottypeE at line 261
/usr/local/go/src/runtime/iface.go in runtime.panicdottypeI at line 271
pkg/sql/sem/tree/eval.go in pkg/sql/sem/tree.(*ColumnAccessExpr).Eval at line 4110
pkg/sql/sem/tree/eval.go in pkg/sql/sem/tree.(*IsNotNullExpr).Eval at line 4356
pkg/sql/execinfrapb/expr.go in pkg/sql/execinfrapb.RunFilter at line 211
pkg/sql/execinfrapb/expr.go in pkg/sql/execinfrapb.(*ExprHelper).EvalFilter at line 200
pkg/sql/rowexec/filterer.go in pkg/sql/rowexec.(*filtererProcessor).Next at line 95
pkg/sql/row_source_to_plan_node.go in pkg/sql.(*rowSourceToPlanNode).Next at line 76
pkg/sql/apply_join.go in pkg/sql.(*applyJoinNode).Next at line 195
pkg/sql/plan_node_to_row_source.go in pkg/sql.(*planNodeToRowSource).Next at line 187
pkg/sql/colexec/columnarizer.go in pkg/sql/colexec.(*Columnarizer).Next at line 222
pkg/sql/colexec/materializer.go in pkg/sql/colexec.(*Materializer).next at line 261
pkg/sql/colexec/materializer.go in pkg/sql/colexec.(*Materializer).nextAdapter at line 286
pkg/sql/colexecerror/error.go in pkg/sql/colexecerror.CatchVectorizedRuntimeError at line 91
pkg/sql/colexec/materializer.go in pkg/sql/colexec.(*Materializer).Next at line 292
pkg/sql/colflow/flow_coordinator.go in pkg/sql/colflow.(*FlowCoordinator).next at line 127
pkg/sql/colflow/flow_coordinator.go in pkg/sql/colflow.(*FlowCoordinator).nextAdapter at line 144
pkg/sql/colexecerror/error.go in pkg/sql/colexecerror.CatchVectorizedRuntimeError at line 91
pkg/sql/colflow/flow_coordinator.go in pkg/sql/colflow.(*FlowCoordinator).Next at line 149
pkg/sql/execinfra/base.go in pkg/sql/execinfra.Run at line 186
pkg/sql/execinfra/processorsbase.go in pkg/sql/execinfra.(*ProcessorBaseNoHelper).Run at line 731
pkg/sql/flowinfra/flow.go in pkg/sql/flowinfra.(*FlowBase).Run at line 447
pkg/sql/colflow/vectorized_flow.go in pkg/sql/colflow.(*vectorizedFlow).Run at line 245
pkg/sql/distsql_running.go in pkg/sql.(*DistSQLPlanner).Run at line 548
pkg/sql/distsql_running.go in pkg/sql.(*DistSQLPlanner).PlanAndRun at line 1289
pkg/sql/apply_join.go in pkg/sql.runPlanInsidePlan at line 318
pkg/sql/recursive_cte.go in pkg/sql.(*recursiveCTENode).Next at line 129
pkg/sql/plan_node_to_row_source.go in pkg/sql.(*planNodeToRowSource).Next at line 187
pkg/sql/colexec/columnarizer.go in pkg/sql/colexec.(*Columnarizer).Next at line 222
pkg/sql/colflow/stats.go in pkg/sql/colflow.(*batchInfoCollector).Next at line 98
Tag Value
Cockroach Release v21.2.7
Cockroach SHA: 37dee54
Platform linux amd64
Distribution CCL
Environment v21.2.7
Command start-single-node
Go Version ``
# of CPUs
# of Goroutines

Jira issue: CRDB-14021

@cockroach-teamcity cockroach-teamcity added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-sentry Originated from an in-the-wild panic report. labels Mar 20, 2022
@yuzefovich yuzefovich changed the title sentry: error.go:88: unexpected error from the vectorized engine: × -- *barriers.barrierError *errutil.withPrefix: unexpected error from the vectorized engine (1) error.go:88: *withstack.withStack (top exception) *assert.withAssertionFailure *colexecerror.notInternalError *colexecerror.notInternalError (check the extra data payloads) sem/tree: v21.2.7: incorrect type conversion in ColumnAccessExpr - maybe DNull check is missing? Mar 22, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 22, 2022
@mgartner mgartner self-assigned this Mar 22, 2022
@mgartner
Copy link
Collaborator

Good eye @yuzefovich! I was able to reverse engineer a reproduction from the clue you left:

defaultdb> CREATE TABLE t (b BOOL);
CREATE TABLE

defaultdb> INSERT INTO t VALUES (false);
INSERT 1

defaultdb> SELECT (CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a from t;
ERROR: internal error: unexpected error from the vectorized engine: interface conversion: tree.Datum is tree.dNull, not *tree.DTuple
SQLSTATE: XX000
DETAIL: stack trace:
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:88: func1()
/usr/local/go/src/runtime/panic.go:965: gopanic()
/usr/local/go/src/runtime/iface.go:261: panicdottypeE()
/usr/local/go/src/runtime/iface.go:271: panicdottypeI()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/sem/tree/eval.go:4110: Eval()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/execinfrapb/expr.go:229: Eval()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:281: ProcessRow()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/execinfra/processorsbase.go:699: ProcessRowHelper()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/rowexec/noop.go:87: Next()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/columnarizer.go:222: Next()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:98: Next()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:239: nextAdapter()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:91: CatchVectorizedRuntimeError()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:243: next()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:282: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:258: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:548: Run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1289: PlanAndRun()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1381: execWithDistSQLEngine()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1072: dispatchToExecutionEngine()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:709: execStmtInOpenState()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:129: execStmt()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1705: func1()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1707: execCmd()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1629: run()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:668: ServeConn()
/go/src/github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:648: func1()
/usr/local/go/src/runtime/asm_amd64.s:1371: goexit()

HINT: You have encountered an unexpected error.

Please check the public issue tracker to check whether this problem is
already tracked. If you cannot find it there, please report the error
with details by creating a new issue.

If you would rather not post publicly, please contact us directly
using the support form.

We appreciate your feedback.

@mgartner
Copy link
Collaborator

Postgres doesn't support our syntax for labelled tuples, but I think this is an equivalent statement that shows that a column access of NULL should result in NULL:

marcus=# CREATE TABLE t (b BOOL);
CREATE TABLE

marcus=# INSERT INTO t VALUES (false);
INSERT 0 1

marcus=# SELECT (CASE WHEN b THEN ((ROW(b)::t)) ELSE NULL END).b FROM t;
  b
------
 NULL
(1 row)

mgartner added a commit to mgartner/cockroach that referenced this issue Mar 24, 2022
Previously, `ColumnAccessExpr.Eval` would panic if the
`ColumnAccessExpr`'s inner expression evaluated to `NULL`, because it
attempted to cast this `NULL` to a `DTuple`. Now, if the inner
expression is `NULL`, `ColumnAccessExpr.Eval` returns `NULL`.

Fixes cockroachdb#78159

Release note (bug fix): A bug has been fixed that caused an internal
error when the inner expression of a column access expression evaluated
to `NULL`. For example, evaluation of the expression
`(CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a` would error when
`b` is `false`. This bug has been present since version 19.1 or earlier.
craig bot pushed a commit that referenced this issue Apr 6, 2022
78423: sql: fix ColumnAccessExpr.Eval with NULL inner expression r=mgartner a=mgartner

#### sql: fix ColumnAccessExpr.Eval with NULL inner expression

Previously, `ColumnAccessExpr.Eval` would panic if the
`ColumnAccessExpr`'s inner expression evaluated to `NULL`, because it
attempted to cast this `NULL` to a `DTuple`. Now, if the inner
expression is `NULL`, `ColumnAccessExpr.Eval` returns `NULL`.

Fixes #78159

Release note (bug fix): A bug has been fixed that caused an internal
error when the inner expression of a column access expression evaluated
to `NULL`. For example, evaluation of the expression
`(CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a` would error when
`b` is `false`. This bug has been present since version 19.1 or earlier.

#### sql: include tuple labels in types returned from typeCheckSameTypedTupleExprs

Previously, an expression that produced a tuple from several potential
values was typed as a tuple without any labels. This prevented a tuple's
column to be accessed by a label name.

For example, the expression below would result in the error "could not
identify column 'a' in record data type".

    SELECT (CASE WHEN true THEN (ROW(1) AS a) ELSE (ROW(2) AS a) END).a

Now, the labels of the first tuple are used in the type of the outer
expression.

Fixes #78515

Release note (bug fix): A bug has been fixed that caused an error when
accessing a named column of a labelled tuple. The bug only occurred when
an expression could produce one of several different tuples. For
example, `(CASE WHEN true THEN (ROW(1) AS a) ELSE (ROW(2) AS a) END).a`
would fail to evaluate. This bug was present since version 22.1.0.
Although present in previous versions, it was impossible to encounter
due to limitations that prevented using tuples in this way.


Co-authored-by: Marcus Gartner <[email protected]>
@craig craig bot closed this as completed in 6b838d9 Apr 6, 2022
@craig craig bot closed this as completed in #78423 Apr 6, 2022
blathers-crl bot pushed a commit that referenced this issue Apr 6, 2022
Previously, `ColumnAccessExpr.Eval` would panic if the
`ColumnAccessExpr`'s inner expression evaluated to `NULL`, because it
attempted to cast this `NULL` to a `DTuple`. Now, if the inner
expression is `NULL`, `ColumnAccessExpr.Eval` returns `NULL`.

Fixes #78159

Release note (bug fix): A bug has been fixed that caused an internal
error when the inner expression of a column access expression evaluated
to `NULL`. For example, evaluation of the expression
`(CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a` would error when
`b` is `false`. This bug has been present since version 19.1 or earlier.
blathers-crl bot pushed a commit that referenced this issue Apr 6, 2022
Previously, `ColumnAccessExpr.Eval` would panic if the
`ColumnAccessExpr`'s inner expression evaluated to `NULL`, because it
attempted to cast this `NULL` to a `DTuple`. Now, if the inner
expression is `NULL`, `ColumnAccessExpr.Eval` returns `NULL`.

Fixes #78159

Release note (bug fix): A bug has been fixed that caused an internal
error when the inner expression of a column access expression evaluated
to `NULL`. For example, evaluation of the expression
`(CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a` would error when
`b` is `false`. This bug has been present since version 19.1 or earlier.
blathers-crl bot pushed a commit that referenced this issue Apr 6, 2022
Previously, `ColumnAccessExpr.Eval` would panic if the
`ColumnAccessExpr`'s inner expression evaluated to `NULL`, because it
attempted to cast this `NULL` to a `DTuple`. Now, if the inner
expression is `NULL`, `ColumnAccessExpr.Eval` returns `NULL`.

Fixes #78159

Release note (bug fix): A bug has been fixed that caused an internal
error when the inner expression of a column access expression evaluated
to `NULL`. For example, evaluation of the expression
`(CASE WHEN b THEN ((ROW(1) AS a)) ELSE NULL END).a` would error when
`b` is `false`. This bug has been present since version 19.1 or earlier.
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
msirek pushed a commit to msirek/cockroach that referenced this issue Sep 8, 2023
This uncomments a unit test from cockroachdb#78159 which was fixed by cockroachdb#109635.

Informs: cockroachdb#109105

Release note: None
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-sentry Originated from an in-the-wild panic report. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants