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: v20.2.3: expected expression to have key #58104

Closed
cockroach-teamcity opened this issue Dec 20, 2020 · 5 comments · Fixed by #61509
Closed

sql: v20.2.3: expected expression to have key #58104

cockroach-teamcity opened this issue Dec 20, 2020 · 5 comments · Fixed by #61509
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.

Comments

@cockroach-teamcity
Copy link
Member

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/2103833235/?referrer=webhooks_plugin

Panic message:

decorrelate_funcs.go:425: expected expression to have key
--
*errutil.leafError: expected expression to have key (1)
decorrelate_funcs.go:425: *withstack.withStack (top exception)
*assert.withAssertionFailure
(check the extra data payloads)

Stacktrace (expand for inline code snippets):

if !ok {
panic(errors.AssertionFailedf("expected expression to have key"))
}
in pkg/sql/opt/norm.(*CustomFuncs).NonKeyCols
https://github.com/cockroachdb/cockroach/blob/89dd2d14c29787378c434ed54937757ef5d9877c/pkg/sql/opt/xform/explorer.og.go#L383-L385 in pkg/sql/opt/xform.(*explorer).exploreSelect
https://github.com/cockroachdb/cockroach/blob/89dd2d14c29787378c434ed54937757ef5d9877c/pkg/sql/opt/xform/explorer.og.go#L19-L21 in pkg/sql/opt/xform.(*explorer).exploreGroupMember
if memberExplored := e.exploreGroupMember(state, member, i); memberExplored {
// No more rules can ever match this expression, so skip it in
in pkg/sql/opt/xform.(*explorer).exploreGroup
// other expressions in this group.
if o.shouldExplore(required) && !o.explorer.exploreGroup(grp).fullyExplored {
fullyOptimized = false
in pkg/sql/opt/xform.(*Optimizer).optimizeGroup
case memo.RelExpr:
state := o.optimizeGroup(t, required)
return state.cost, state.fullyOptimized
in pkg/sql/opt/xform.(*Optimizer).optimizeExpr
// Optimize the child with respect to those properties.
childCost, childOptimized := o.optimizeExpr(member.Child(i), childRequired)
in pkg/sql/opt/xform.(*Optimizer).optimizeGroupMember
// Optimize the group member with respect to the required properties.
memberOptimized := o.optimizeGroupMember(state, member, required)
in pkg/sql/opt/xform.(*Optimizer).optimizeGroup
rootProps := o.mem.RootProps()
o.optimizeGroup(root, rootProps)
in pkg/sql/opt/xform.(*Optimizer).Optimize
opc.log(ctx, "optimizing (no placeholders)")
if _, err := opc.optimizer.Optimize(); err != nil {
return nil, err
in pkg/sql.(*optPlanningCtx).buildReusableMemo
memo, err := opc.buildReusableMemo(ctx)
if err != nil {
in pkg/sql.(*planner).prepareUsingOptimizer
// future.
flags, err := p.prepareUsingOptimizer(ctx)
if err != nil {
in pkg/sql.(*connExecutor).populatePrepared
p.semaCtx.Annotations = tree.MakeAnnotations(stmt.NumAnnotations)
flags, err = ex.populatePrepared(ctx, txn, placeholderHints, p)
return err
in pkg/sql.(*connExecutor).prepare.func1

cockroach/pkg/kv/db.go

Lines 706 to 708 in 89dd2d1

err := txn.exec(ctx, func(ctx context.Context, txn *Txn) error {
return retryable(ctx, txn)
})
in pkg/kv.(*DB).Txn.func1

cockroach/pkg/kv/txn.go

Lines 810 to 812 in 89dd2d1

}
err = fn(ctx, txn)
in pkg/kv.(*Txn).exec

cockroach/pkg/kv/db.go

Lines 705 to 707 in 89dd2d1

txn.SetDebugName("unnamed")
err := txn.exec(ctx, func(ctx context.Context, txn *Txn) error {
return retryable(ctx, txn)
in pkg/kv.(*DB).Txn
// than bubbling them up to the connExecutor state machine.
if err := ex.server.cfg.DB.Txn(ctx, prepare); err != nil {
return nil, err
in pkg/sql.(*connExecutor).prepare
// Prepare the query. This completes the typing of placeholders.
prepared, err := ex.prepare(ctx, stmt, placeholderHints, origin)
if err != nil {
in pkg/sql.(*connExecutor).addPreparedStmt
ps, err := ex.addPreparedStmt(
ctx,
in pkg/sql.(*connExecutor).execPrepare
stmtCtx := withStatement(ctx, ex.curStmt)
ev, payload = ex.execPrepare(stmtCtx, tcmd)
case DescribeStmt:
in pkg/sql.(*connExecutor).execCmd
var err error
if err = ex.execCmd(ex.Ctx()); err != nil {
if errors.IsAny(err, io.EOF, errDrainingComplete) {
in pkg/sql.(*connExecutor).run
}()
return h.ex.run(ctx, s.pool, reserved, cancel)
}
in pkg/sql.(*Server).ServeConn
reservedOwned = false // We're about to pass ownership away.
retErr = sqlServer.ServeConn(ctx, connHandler, reserved, cancelConn)
}()
in pkg/sql/pgwire.(*conn).processCommandsAsync.func1
/usr/local/go/src/runtime/asm_amd64.s#L1356-L1358 in runtime.goexit

pkg/sql/opt/norm/decorrelate_funcs.go in pkg/sql/opt/norm.(*CustomFuncs).NonKeyCols at line 425
pkg/sql/opt/xform/explorer.og.go in pkg/sql/opt/xform.(*explorer).exploreSelect at line 384
pkg/sql/opt/xform/explorer.og.go in pkg/sql/opt/xform.(*explorer).exploreGroupMember at line 20
pkg/sql/opt/xform/explorer.go in pkg/sql/opt/xform.(*explorer).exploreGroup at line 178
pkg/sql/opt/xform/optimizer.go in pkg/sql/opt/xform.(*Optimizer).optimizeGroup at line 463
pkg/sql/opt/xform/optimizer.go in pkg/sql/opt/xform.(*Optimizer).optimizeExpr at line 250
pkg/sql/opt/xform/optimizer.go in pkg/sql/opt/xform.(*Optimizer).optimizeGroupMember at line 505
pkg/sql/opt/xform/optimizer.go in pkg/sql/opt/xform.(*Optimizer).optimizeGroup at line 450
pkg/sql/opt/xform/optimizer.go in pkg/sql/opt/xform.(*Optimizer).Optimize at line 224
pkg/sql/plan_opt.go in pkg/sql.(*optPlanningCtx).buildReusableMemo at line 409
pkg/sql/plan_opt.go in pkg/sql.(*planner).prepareUsingOptimizer at line 126
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).populatePrepared at line 252
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).prepare.func1 at line 194
pkg/kv/db.go in pkg/kv.(*DB).Txn.func1 at line 707
pkg/kv/txn.go in pkg/kv.(*Txn).exec at line 811
pkg/kv/db.go in pkg/kv.(*DB).Txn at line 706
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).prepare at line 206
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).addPreparedStmt at line 120
pkg/sql/conn_executor_prepare.go in pkg/sql.(*connExecutor).execPrepare at line 66
pkg/sql/conn_executor.go in pkg/sql.(*connExecutor).execCmd at line 1551
pkg/sql/conn_executor.go in pkg/sql.(*connExecutor).run at line 1391
pkg/sql/conn_executor.go in pkg/sql.(*Server).ServeConn at line 508
pkg/sql/pgwire/conn.go in pkg/sql/pgwire.(*conn).processCommandsAsync.func1 at line 626
/usr/local/go/src/runtime/asm_amd64.s in runtime.goexit at line 1357
Tag Value
Cockroach Release v20.2.3
Cockroach SHA: 89dd2d1
Platform linux amd64
Distribution CCL
Environment v20.2.3
Command server
Go Version ``
# of CPUs
# of Goroutines
@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 Dec 20, 2020
@yuzefovich yuzefovich changed the title sentry: decorrelate_funcs.go:425: expected expression to have key -- *errutil.leafError: expected expression to have key (1) decorrelate_funcs.go:425: *withstack.withStack (top exception) *assert.withAssertionFailure (check the extra data payloads) sql: v20.2.3: expected expression to have key Dec 21, 2020
@RaduBerinde
Copy link
Member

The rule is SplitDisjunctionAddKey. It is calling NonKeyCols on leftScan. It looks like a copy/paste-o from the other variant, but I'm surprised it wasn't caught by any tests.

@RaduBerinde
Copy link
Member

Ah, my bad, leftScan is created by the rule and is a scan that contains the PK.. This is strange. I wonder if maybe the table is virtual and the PK stuff doesn't work as intended.

@RaduBerinde RaduBerinde assigned RaduBerinde and unassigned mgartner Jan 11, 2021
@RaduBerinde
Copy link
Member

Indeed, I was able to repro with:

select * from pg_catalog.pg_attrdef WHERE (adnum = 1 AND adrelid = 1) OR (adbin = 'foo' AND adrelid = 2);

The strange filter is necessary to trick the rule into firing (there is a single index on adrelid).

@RaduBerinde
Copy link
Member

Interestingly, this doesn't seem to be a problem on master, where we have changed virtual tables to always present a primary index.

@RaduBerinde
Copy link
Member

Actually, looks like this was already fixed (on master and release-20.2) by #58434 - we no longer call NonKeyCols at all. I will add some regression tests.

craig bot pushed a commit that referenced this issue Mar 5, 2021
61359: tracing: use byte-limits for logs/structured events per span r=irfansharif a=irfansharif

Touches #59188. Follow-on work from #60678. We can introduce byte-limits for
verbose logging and structured events, instead of limiting things based on
count.

This PR also:
- adds a _dropped tag to recordings with dropped logs/structured events.
- squashes a bug where reset spans (as used in SessionTracing) still
  held onto earlier structured events
- moves away from the internal usage of the opentracing.LogRecord type,
  it's unnecessary

Release justification: low risk, high benefit changes to existing
functionality

Release note: None

---

+cc @knz / @erikgrinaker / @angelapwen for pod-visibility.

61482: jobs: add job metrics per-type to track success, failure, and cancel r=fqazi a=fqazi

Fixes: #59711

Previously, there were only over all counters tracking how many
jobs were completed, cancelled, or failed. This was inadequate
because it didn't make it easy to tell in aggregate what job
types they were. To address this, this patch will add counters
for different job types for tracking success, failure, and
cancellation.

Release justification: Low risk change only adding a metric inside
the crdb_internal.feature_usage table
Release note: None

61491: sqlsmith: add support for computed columns r=RaduBerinde a=RaduBerinde

This changes the random table generator to also create computed
columns (either STORED or VIRTUAL). Some example of definitions:
 - `col1_14 STRING NOT NULL AS (lower(CAST(col1_8 AS STRING))) VIRTUAL`
 - `col2_6 DECIMAL NOT NULL AS (col2_2 + 1:::DECIMAL) STORED`
 - `col1_13 INT4 AS (col1_0 + col1_10) STORED`

Release justification: non-production code change.

Release note: None

Informs #57608.

61509: sql: add a regression test r=RaduBerinde a=RaduBerinde

This commit adds a regression test for #58104 (the problem was already
fixed).

Resolves #58104.

Release justification: non-production code change.

Release note: None

61522: opt: fix fetch scope in UPDATE..FROM statements r=mgartner a=mgartner

Previously, the fetch scope incorrectly included columns in the FROM
clause of an UPDATE..FROM statement. As a result, column names shared by
the FROM clause and the mutating table lead to ambiguity when resolving
partial index DEL column expressions. This commit ensures that the fetch
scope does not include columns in the FROM clause.

Fixes #61284

Release justification: This is a low-risk bug fix to existing
functionality.

Release note (bug fix): An UPDATE..FROM statement where the FROM clause
contained column names that match table column names erred if the table
had a partial index predicate referencing those columns. This bug,
present since partial indexes were released in version 20.2.0, has been
fixed.

61553: ccl,server: error.Wrap on previously handled errors r=[dt,miretskiy] a=stevendanna

These errors.Wrap calls are wrapping errors that are nil and thus will
always return a nil error.

Release justification: Minor error handling fixes
Release note: None

Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
@craig craig bot closed this as completed in b97e82c Mar 5, 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. O-sentry Originated from an in-the-wild panic report.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants