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: when a table drops a bunch of columns saved gists will runtime panic on decode #76800

Closed
cucaroach opened this issue Feb 19, 2022 · 0 comments · Fixed by #76801
Closed
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-queries SQL Queries Team

Comments

@cucaroach
Copy link
Contributor

select crdb_internal.decode_plan_gist('AgH2AQIA/wMAAAADBxQFFCH2AQAA');
ERROR: internal error: runtime error: index out of range [6] with length 6
SQLSTATE: XX000
DETAIL: stack trace:
github.com/cockroachdb/cockroach/pkg/util/errorutil/catch.go:29: ShouldCatch()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/result_columns.go:33: func1()
runtime/panic.go:1047: gopanic()
runtime/panic.go:90: goPanicIndex()
github.com/cockroachdb/cockroach/pkg/sql/opt_catalog.go:1063: Column()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/result_columns.go:204: tableColumns()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/result_columns.go:57: getResultColumns()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/explain_factory.go:89: newNode()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/plan_gist_factory.og.go:1081: decodeOperatorBody()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/plan_gist_factory.go:220: decodeOp()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/plan_gist_factory.go:193: DecodePlanGistToPlan()
github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain/plan_gist_factory.go:145: DecodePlanGistToRows()
github.com/cockroachdb/cockroach/pkg/sql/plan_opt.go:661: DecodeGist()
github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/generator_builtins.go:473: Start()
github.com/cockroachdb/cockroach/pkg/sql/rowexec/project_set.go:158: nextInputRow()
github.com/cockroachdb/cockroach/pkg/sql/rowexec/project_set.go:241: Next()
github.com/cockroachdb/cockroach/pkg/sql/colexec/columnarizer.go:229: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/stats.go:99: Next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:240: nextAdapter()
github.com/cockroachdb/cockroach/pkg/sql/colexecerror/error.go:91: CatchVectorizedRuntimeError()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:244: next()
github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:287: Run()
github.com/cockroachdb/cockroach/pkg/sql/colflow/vectorized_flow.go:259: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:574: Run()
github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1410: PlanAndRun()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1420: execWithDistSQLEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1097: dispatchToExecutionEngine()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:727: execStmtInOpenState()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:140: execStmt()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1782: func1()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1786: execCmd()
github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1715: run()
@cucaroach cucaroach added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 19, 2022
@cucaroach cucaroach self-assigned this Feb 19, 2022
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Feb 19, 2022
cucaroach added a commit to cucaroach/cockroach that referenced this issue Feb 19, 2022
Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: cockroachdb#76800

Release note: None
craig bot pushed a commit that referenced this issue Feb 23, 2022
76606: spanconfigsqltranslator: emit all SystemTarget span configs when required r=arulajmani a=adityamaru

This change teaches the SQLTranslator to emit SpanConfigurations
corresponding to `spanconfig.SystemTargets`. Today, these SpanConfigurations
only contain ProtectionPolicies, corresponding to protected timestamp
records that may be written by the host tenant to protect its cluster,
a secondary tenant to protect its cluster, or the host tenant to protect
a secondary tenant.

The SystemTarget span configurations will be applied to a SystemSpanConfig
store that will be introduced in a follow up PR.

Informs: #73727

Release note: None

76801: sql: fix cached gists panic'ing after schema changes r=mgartner a=cucaroach

Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: #76800

Release note: None


76900: mon: when creating inherited BytesMonitor don't copy metrics r=yuzefovich a=aliher1911

Previously monitor inherited parameters like name, resource, limits etc
from parent and also included metrics.
This is not good as metrics were updated twice when borrowing resources,
one time by child monitor and again by parent monitor which lead to
misreporting.
This patch removes metrics from inheritance. It is safe to do as metrics
could be safely set to nil. It could lead to some memory that is reserved
by child pool reported as allocated, but it is better than doing x2.

Release note: None

Fixes #76898

Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>
@craig craig bot closed this as completed in edc105f Feb 23, 2022
maryliag pushed a commit to maryliag/cockroach that referenced this issue Feb 28, 2022
Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: cockroachdb#76800

Release note: None
RajivTS pushed a commit to RajivTS/cockroach that referenced this issue Mar 6, 2022
Previously we didn't guard against out of bounds column ids in gists,
if a gist is created on a table and then that table drops columns we
would hit a runtime index out of bounds panic. By design gist decoding
is best effort and should silently ignore these columns when this
happens. The columns are used for equality conditions, ie:

             └── • lookup join
                 │ table: broker@broker_pkey
                 │ equality: (tr_s_symb) = (b_id)

So they are a nice to have. Doing anything more sophisticated would
dramatically increase the gist size so isn't worth it.

Fixes: cockroachdb#76800

Release note: None
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
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. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant