Skip to content

colexec: cfetcher errors on setup when indexed column contains unhandled type#42999

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rohany:vec-error-unsupported
Dec 5, 2019
Merged

colexec: cfetcher errors on setup when indexed column contains unhandled type#42999
craig[bot] merged 1 commit intocockroachdb:masterfrom
rohany:vec-error-unsupported

Conversation

@rohany
Copy link
Copy Markdown
Contributor

@rohany rohany commented Dec 5, 2019

Fixes #42994.

Indexed columns are always decoded by the fetcher, even if they are unneeded.
This PR adds the check to cfetcher initialization to error out if an indexed
column is of an unneeded type.

Release note (bug fix): Fixed a bug where scanning an index of an unsupported
type with the vectorized engine would lead to an internal error.

@rohany rohany requested review from a team and yuzefovich December 5, 2019 19:19
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Thanks!

cc @asubiotto for the backport

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Dec 5, 2019

bors r=yuzefovich

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 5, 2019

Build failed (retrying...)

@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Dec 5, 2019

bors r-

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 5, 2019

Canceled

…led type

Fixes cockroachdb#42994.

Indexed columns are always decoded by the fetcher, even if they are unneeded.
This PR adds the check to cfetcher initialization to error out if an indexed
column is of an unneeded type.

Release note (bug fix): Fixed a bug where scanning an index of an unsupported
type with the vectorized engine would lead to an internal error.
@rohany rohany force-pushed the vec-error-unsupported branch from 5b8932b to e6e9f0b Compare December 5, 2019 21:16
@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Dec 5, 2019

bors r=yuzefovich

craig bot pushed a commit that referenced this pull request Dec 5, 2019
42250: storage/engine: introduce a datadriven framework to run MVCC tests r=itsbilal,petermattis a=knz

Previously MVCC tests were hand-coded in Go.
This was making it hard(er) to introduce new tests or modify existing
tests.

This commit improves upon this situation by introducing a new
datadriven test, `TestMVCCHistories`, which runs MVCC
tests written using a DSL:

```
begin_txn      t=<name> [ts=<int>[,<int>]]
remove_txn     t=<name>
resolve_intent t=<name> k=<key> [status=<txnstatus>]
restart_txn    t=<name>
update_txn     t=<name> t2=<name>
step_txn       t=<name> [n=<int>]
advance_txn    t=<name> ts=<int>[,<int>]
txn_status     t=<name> status=<txnstatus>

check_intent   k=<key> [none]

put       [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key> v=<string> [raw]
cput      [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key> v=<string> [raw] [cond=<string>]
increment [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key> [inc=<val>]
del       [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key>
get       [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key> [inconsistent] [tombstones]
scan      [t=<name>] [ts=<int>[,<int>]] [resolve] k=<key> [end=<key>] [inconsistent] [tombstones] [reverse]

merge     [ts=<int>[,<int>]] k=<key> v=<string> [raw]

clear_range    k=<key> end=<key>
```

Where `<key>` can be a simple string, or a string
prefixed by the following characters:

- `=foo` means exactly key `foo`
- `+foo` means `Key(foo).Next()`
- `-foo` means `Key(foo).PrefixEnd()`

Additionally, the pseudo-command `with` enables sharing
a group of arguments between multiple commands, for example:

```
with t=A
   begin_txn
   with k=a
     put v=b
     resolve_intent
```

Release note: None

42999: colexec: cfetcher errors on setup when indexed column contains unhandled type r=yuzefovich a=rohany

Fixes #42994.

Indexed columns are always decoded by the fetcher, even if they are unneeded.
This PR adds the check to cfetcher initialization to error out if an indexed
column is of an unneeded type.

Release note (bug fix): Fixed a bug where scanning an index of an unsupported
type with the vectorized engine would lead to an internal error.

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 5, 2019

Build succeeded

@craig craig bot merged commit e6e9f0b into cockroachdb:master Dec 5, 2019
@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Dec 6, 2019

On second thought, I'm wondering if #43002 allows this test to pass without restricting the types of queries that the vectorized engine can run...

@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Dec 6, 2019

I think we should revert this change if #43024 goes through. It is too restrictive and doesn't allow the cfetcher to skip columns in keys when decoding.

@yuzefovich
Copy link
Copy Markdown
Member

There was this failure with sqlsmith #42217 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql: internal error when CFetcher reads from an index with unneeded unsupported type

3 participants