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

opt: audit correlated subquery code for panic catching #98786

Closed
mgartner opened this issue Mar 16, 2023 · 3 comments · Fixed by #99835
Closed

opt: audit correlated subquery code for panic catching #98786

mgartner opened this issue Mar 16, 2023 · 3 comments · Fixed by #99835
Assignees
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) GA-blocker T-sql-queries SQL Queries Team

Comments

@mgartner
Copy link
Collaborator

mgartner commented Mar 16, 2023

Audit the newly added code in the execbuilder for missing panic-catchers. Look for issues similar to the one fixed in this commit: 5c0a632

From the v23.1 pre-mortem.

Jira issue: CRDB-25521

@mgartner mgartner added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) GA-blocker labels Mar 16, 2023
@blathers-crl
Copy link

blathers-crl bot commented Mar 16, 2023

Hi @mgartner, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 16, 2023
@mgartner mgartner added the branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 label Mar 16, 2023
@DrewKimball
Copy link
Collaborator

I'm going to see if I can add some panic injection in the optimizer for our tests this week.

@mgartner
Copy link
Collaborator Author

Thanks @DrewKimball. That's tracked here: #98787

craig bot pushed a commit that referenced this issue Mar 31, 2023
99312: sqlsmith: add DEFAULT expressions to newly added columns r=mgartner a=mgartner

Sqlsmith now builds `ALTER TABLE .. ADD COLUMN .. DEFAULT` statements
with default expressions that have different types than the column type.
This is allowed if the default expression type can be assignment-casted
to the column's type.

Fixes #98133

Release note: None


99348: testutils: move default test tenant message r=rharding6373 a=herkolategan

In order to reduce logging noise but still inform test authors of the default test tenant, the message has been moved to where there is a `testing.TB` interface.

Epic: CRDB-18499

99835: opt/execbuilder: add panic catching to buildRoutinePlanGenerator r=mgartner a=mgartner

This commit adds a panic catcher to callback functions created in
execbuilder and invoked during evaluation of UDFs and correlated
subqueries. It matches the panic catcher logic in `buildApplyJoin`.

Fixes #98786

Release note: None


100267: roachtest: own autoupgrade to TestEng r=renatolabs a=tbg

Discussed in #99479.

Epic: none
Release note: None


100286: roachtest: prevent aws roachtest panic r=rail a=msbutler

After #99723 merged as a bandaid for #98783, the aws roachtest nightly began to panic because of a different roachtest papercut #96655. Specifically, because roachtest filters which tests run on which cloud within the evaluation of the test closure, tests meant to run on gce will still get registered in an AWS run. During the registration of the gce test
`restore/tpce/400GB/gce/nodes=4/cpus=8/lowmem` _on aws_, the aws test harness panics because the aws roachprod implementation does not have a low memory cpu configuration. This patch prevents this panic and should be reverted once the pr #99402 merges.

Epic: None

Release note: None

100294: tenantcapabilitiestestutils: add a missing default case r=ajwerner a=ajwerner

The test should fail if we ever add a new type of capability and use it in the data driven test but don't update the test to handle it.

Epic: none

Follow-up from #100217 (review)

Release note: None

100296: rpc: correctly check for nil before cast r=ajwerner a=andrewbaptist

As part of the fix of #99104, a cast without a nil check was introduced. This PR addresses that by only casting if it is known to be not nil.

Epic: none
Fixes: #100275
Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: ajwerner <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
@craig craig bot closed this as completed in e14df21 Mar 31, 2023
blathers-crl bot pushed a commit that referenced this issue Mar 31, 2023
This commit adds a panic catcher to callback functions created in
execbuilder and invoked during evaluation of UDFs and correlated
subqueries. It matches the panic catcher logic in `buildApplyJoin`.

Fixes #98786

Release note: None
craig bot pushed a commit that referenced this issue Apr 3, 2023
…0440

98267: kvserver: replica load distribution metric r=andrewbaptist a=kvoli

This PR adds two histograms, tracking the percentiles of replica CPU time and replica Batch requests received. Previously, there was only point in time insight into either distribution via hotranges. This change enables historical timeseries tracking via the metric `rebalancing.replicas.cpunanospersecond` for CPU and `rebalancing.replicas.queriespersecond` for Batch Requests.

Informs: #98255

98690: upgrades: make the schema telemetry job setup permanent r=ajwerner a=ajwerner

This migration should run for every cluster.

Epic: none

Informs: #96763

Release note: None

98912: sql: address a few nits from reviewing an old PR r=yuzefovich a=yuzefovich

This commit addresses several nits (a typo, missing periods, and precisely allocating a slice) that I noticed while reviewing the old PR which introduced DELETE FROM ... USING support.

Epic: None

Release note: None

99981: opt: remove panics in execbuilder r=mgartner a=mgartner

This commit replaces `panic`s in execbuilder with returned errors. This
is safer because execbuilder functions are invoked outside the
panic-recoverable `Optimizer.Optimize` function.

Informs #98786

Release note: None


100312: bazel: upgrade to bazel 5.4.0 r=rail a=rickystewart

Release note: None
Epic: none

100321: streamingccl: clean up stream ingestion job execution r=stevendanna a=msbutler

This patch cleans up the stream ingestion job code by:
1. Seperating dist sql processor planning and execution in seperate functions.
2. Setting up the processor planning code for a future with job replanning.
3. Seperating stream ingestion and completion into seperat functions.
3. Cleaning up a few log lines and error messages.

Epic: none

Release note: none

100349: kvserver: nudge replicate queue on span config update r=irfansharif,knz a=arulajmani

We eagerly enqueue replicas into the split/merge queues whenever there is a span config update. A span config update could also imply a need to {up,down}replicate as well. This patch actively enqueues overlapping replicas into the replicate queue as well.

Epic: none

Release note: None

100355: sql: fix read-only SSL var r=knz a=rafiss

The variable will now look at the connection state to determine if SSL/TLS is being used, rather than relying on server configuration params, which aren't sufficient to be able to determine the type of connection.

fixes #99606
Release note: None

100440: server, sql: show in-memory data when no data is persisted r=maryliag a=maryliag

Fixes #100439

When no data is persisted to sql stats tables (because no flush happened yet or because the flush is disabled), the endpoints should fall back to the combined view that contains the in-memory data.

https://www.loom.com/share/b5e3a227a9904a19a5279ed828a3b3f3

Release note (sql change): When there is no data persisted, show the in-memory data.

Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: ajwerner <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: maryliag <[email protected]>
@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
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) GA-blocker T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants