sql,colexec: preliminary changes for wrapping LocalPlanNode#57644
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Dec 8, 2020
Merged
sql,colexec: preliminary changes for wrapping LocalPlanNode#57644craig[bot] merged 2 commits intocockroachdb:masterfrom
craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
This commit modifies several internal queries for SHOW commands to make the ordering of the output deterministic. The following had been augmented: - SHOW PARTITIONS - SHOW CREATE - SHOW INDEXES - SHOW COLUMNS (this also had an addition of ORDER BY clause of array_agg that computes the set of indexes) - SHOW CONSTRAINTS. Additionally, several logic tests were adjusted to use `rowsort` directive when ORDER BY isn't specified in the query as well as some EXPLAIN tests were moved into the execbuilder ones. This work was prompted by the fact that the vectorized engine produces the output for those queries with a different order (which is valid given omitted ORDER BY clauses before) than the row-by-row engine. Release note (sql change): Several SHOW commands have been adjusted to enforce a particular ordering of the output rows.
Previously, we would use `Int64` canonical type family to represents Oids. However, such conversion is lossful because we're losing some information (like semantic type and resolvable name), so if we were to do `columnarize -> materialize` conversion on an Oid, we wouldn't get the same thing back in the general case. To go around this issue this commit switches Oid type family to be handled by the datum-backed type. This will have some performance hit, but the queries with Oids aren't expected to process a lot of data, so that hit could be safely ignored. Release note: None
Member
|
This change is |
asubiotto
approved these changes
Dec 8, 2020
Contributor
asubiotto
left a comment
There was a problem hiding this comment.
Reviewed 26 of 26 files at r1, 10 of 10 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained
Member
Author
|
TFTR! bors r+ |
Contributor
|
Build succeeded: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sql: order the output of some SHOW queries deterministically
This commit modifies several internal queries for SHOW commands to make
the ordering of the output deterministic. The following had been
augmented:
array_agg that computes the set of indexes)
Additionally, several logic tests were adjusted to use
rowsortdirective when ORDER BY isn't specified in the query as well as some
EXPLAIN tests were moved into the execbuilder ones.
This work was prompted by the fact that the vectorized engine produces
the output for those queries with a different order (which is valid
given omitted ORDER BY clauses before) than the row-by-row engine.
Release note (sql change): Several SHOW commands have been adjusted to
enforce a particular ordering of the output rows.
colexec: use datum-backed type for Oids
Previously, we would use
Int64canonical type family to representsOids. However, such conversion is lossful because we're losing some
information (like semantic type and resolvable name), so if we were to
do
columnarize -> materializeconversion on an Oid, we wouldn't getthe same thing back in the general case. To go around this issue this
commit switches Oid type family to be handled by the datum-backed type.
This will have some performance hit, but the queries with Oids aren't
expected to process a lot of data, so that hit could be safely ignored.
Release note: None