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.

Sign up for GitHub

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

Jump to bottom

sql: panic in row fetcher during UPSERT #40410

Closed
sentry-io bot opened this issue Sep 2, 2019 · 3 comments · Fixed by #40844
Closed

sql: panic in row fetcher during UPSERT #40410

sentry-io bot opened this issue Sep 2, 2019 · 3 comments · Fixed by #40844
Assignees
@rohany
Labels
A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. 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. S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting.

Comments

@sentry-io
Copy link

sentry-io bot commented Sep 2, 2019

Sentry Issue: COCKROACHDB-11N

Version 19.1.3

UPSERT INTO _(_, _, _, _, _, _, _, _, _, _, _, _, _, _) VALUES (_, _, __more10__), (__more4__)
*log.safeError: conn_executor.go:434: panic while executing 1 statements: UPSERT INTO _(_, _, _, _, _, _, _, _, _, _, _, _, _, _) VALUES (_, _, __more10__), (__more4__): caused by <redacted>
  File "github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go", line 434, in func1
  File "github.com/cockroachdb/cockroach/pkg/sql/row/fetcher.go", line 768, in processKV
  File "github.com/cockroachdb/cockroach/pkg/sql/row/fetcher.go", line 1062, in NextRow
  File "github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/tablereader.go", line 165, in Next
  File "github.com/cockroachdb/cockroach/pkg/sql/distsqlrun/joinreader.go", line 413, in performLookup
...
(25 additional frame(s) were not displayed)

conn_executor.go:434: panic while executing 1 statements: UPSERT INTO _(_, _, _, _, _, _, _, _, _, _, _, _, _, _) VALUES (_, _, __more10__), (__more4__): caused by <redacted>
@knz knz added A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting. labels Sep 2, 2019
@knz
Copy link
Contributor

knz commented Sep 2, 2019

cc @jordanlewis for triage

@knz knz added the O-sentry Originated from an in-the-wild panic report. label Sep 2, 2019
@solongordon solongordon assigned rohany and unassigned solongordon Sep 10, 2019
@rohany
Copy link
Contributor

rohany commented Sep 11, 2019

Its tough to see whats going on without being able to reproduce this, but the panic comes from here --

for idx, ok := table.neededValueColsByIdx.Next(0); ok; idx, ok = table.neededValueColsByIdx.Next(idx + 1) {
			table.row[idx].UnsetDatum()
		}

We get a panic by indexing out of bounds in row. Row is already initialized to be an array with length of the table's columns, so the error is coming from the row fetcher being given a bad set of "needed columns" which contain a value larger than the number of columns in the table. @jordanlewis based on this info, what should we do here?

@rohany
Copy link
Contributor

rohany commented Sep 11, 2019 •
edited
Loading

^ the above code is from the row fetcher (line 795 on master)

@rohany rohany mentioned this issue Sep 17, 2019
Merged
craig bot pushed a commit that referenced this issue Sep 19, 2019
Merge #40844
0370137 
40844: row: Add a check for integrity of neededValueColsByIdx in row fetcher r=jordanlewis a=rohany

The panic #40410 arises when a value in neededValueColsByIdx is out of
bounds of the row buffer used in the fetcher. The length of the row
buffer is equal to the number of columns in the table, so this panic is
most likely caused by the neededValueColsByIdx containing an invalid
column ID. To guard against this / have more information about debugging
this in the future, we verify during fetcher setup that all the values
in neededValueColsByIdx are within range of the table's row buffer.

Fixes #40410.

Release justification: Low risk assertion of valid behavior.

Release note: None

Co-authored-by: Rohan Yadav <[email protected]>
@craig craig bot closed this as completed in 5bdc5c6 Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rohany rohany

Labels
A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. 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. S-2-temp-unavailability Temp crashes or other availability problems. Can be worked around or resolved by restarting.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

row: Add a check for integrity of neededValueColsByIdx in row fetcher rohany/cockroach
3 participants
@knz @solongordon @rohany