Skip to content

release-19.2: colexec: skip over unneeded columns with unknown types in cfetcher#42934

Merged
asubiotto merged 2 commits intocockroachdb:release-19.2from
asubiotto:backport19.2-42616
Dec 7, 2019
Merged

release-19.2: colexec: skip over unneeded columns with unknown types in cfetcher#42934
asubiotto merged 2 commits intocockroachdb:release-19.2from
asubiotto:backport19.2-42616

Conversation

@asubiotto
Copy link
Copy Markdown
Contributor

Previously, we would return an error if we encountered an unknown type in the
cfetcher, even if the values from the column were never decoded into the output
coldata.Batch. This change supports reading from a table with unknown types as
long as those columns are marked as not needed. The approach this patch takes
is to allow a coldata.Batch to be created with unknown types, but disallowing
any uses of unknown type Vecs, other than observing that it is of unknown type.

Release note (sql change): Vectorized queries that execute only on supported
types even if those types form part of a table with unsupported types are now
run through the vectorized engine. This would previously fall back to the
row-by-row execution engine.

@asubiotto asubiotto requested review from a team and yuzefovich December 4, 2019 10:03
@asubiotto asubiotto requested a review from a team as a code owner December 4, 2019 10:03
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@yuzefovich
Copy link
Copy Markdown
Member

[nit]: release-19.2 is missing from the title of the PR.

Also, there is a related test failure (#37815 (comment)), so I would hold off on backporting until we figure out the cause of that issue. I'll dig in today.

@asubiotto
Copy link
Copy Markdown
Contributor Author

Thanks, will wait for that to merge and add it to this PR

@asubiotto asubiotto changed the title colexec: skip over unneeded columns with unknown types in cfetcher release-19.2: colexec: skip over unneeded columns with unknown types in cfetcher Dec 6, 2019
asubiotto and others added 2 commits December 6, 2019 11:11
Previously, we would return an error if we encountered an unknown type in the
cfetcher, even if the values from the column were never decoded into the output
coldata.Batch. This change supports reading from a table with unknown types as
long as those columns are marked as not needed. The approach this patch takes
is to allow a coldata.Batch to be created with unknown types, but disallowing
any uses of unknown type Vecs, other than observing that it is of unknown type.

Release note (sql change): Vectorized queries that execute only on supported
types even if those types form part of a table with unsupported types are now
run through the vectorized engine. This would previously fall back to the
row-by-row execution engine.
We recently merged a change to allow for 'unknown' vectors to be present
among colvecs. However, we forgot to update the resetting of the batch
- unknown columns do not have nulls and don't need to be reset. Now this
is fixed.

Also this commit removes an unused field in cfetcher.

Release note: None
@asubiotto
Copy link
Copy Markdown
Contributor Author

Backporting 656b636 as well. Will wait for #43002 because I think we probably need that as well.

@asubiotto
Copy link
Copy Markdown
Contributor Author

Actually, we should backport this first and then @rohany's changes

@asubiotto asubiotto requested a review from rohany December 6, 2019 22:15
@rohany
Copy link
Copy Markdown
Contributor

rohany commented Dec 7, 2019

LGTM

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:

Reviewed 5 of 8 files at r1, 3 of 3 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rohany)

@asubiotto asubiotto merged commit c85d435 into cockroachdb:release-19.2 Dec 7, 2019
@asubiotto asubiotto deleted the backport19.2-42616 branch December 21, 2019 12:33
@asubiotto asubiotto restored the backport19.2-42616 branch January 15, 2020 16:42
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.

4 participants