colbuilder: vectorizing rendering on top of wrapped processors #83689
Conversation
|
This change is |
This commit removes no longer used `OriginalOutputColumns` and `OriginalRenderExprs` fields from the `PostProcessSpec` - they are no longer needed because we now propagate the set of needed columns in the `IndexFetchSpec`. Release note: None
This commit is just a mechanical change. Release note: None
yuzefovich
force-pushed
the
vectorized-rendering
branch
from
c3e1459 to
c4ab930
Compare
yuzefovich
changed the title
Previously, whenever we needed to wrap a row-by-row processor into the vectorized flow, we would pass in the whole `PostProcessSpec` to that processor. When this was originally implemented several years ago, this was needed so that processors could determine the set of "needed columns" (which should be decoded from KV responses). However, recently we refactored that, and now the set of needed columns is passed via the `IndexFetchSpec`. This means that we no longer need to pass the whole `PostProcessSpec` when wrapping, and this commit takes advantage of that observation in order to vectorize the evaluation of render expressions. In particular, vectorizing of render expressions allows us to plan more efficient vectorized builtins. We still pass all other parts of the `PostProcessSpec` (meaning that the wrapped processor is still responsible for projections, limits, and offsets) since those operations will limit the number of datums that need to be converted to the columnar in-memory format as well as provide limit hints to the wrapped processors. Release note: None
yuzefovich
force-pushed
the
vectorized-rendering
branch
from
c4ab930 to
f5acd81
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 1 of 3 files at r3.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)
pkg/sql/opt/exec/execbuilder/testdata/tpch_vec line 20803 at r3 (raw file):
│ └ *colexecprojconst.projNEBytesBytesConstOp └ *colexecbase.constInt64Op └ *colexec.bufferOp
Why did this change so much?
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/tpch_vec line 20803 at r3 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
Why did this change so much?
Previously CASE statement was executed via the row-by-row engine as part of the PostProcessSpec of joinReader and now we vectorize it. caseOp has many branches and they are partially shown in EXPLAIN (VEC) diagram, and here we actually have a caseOp that is an input to another caseOp.
There was a problem hiding this comment.
PostProcessSpec change, but otherwise this looks good.
Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status:
pkg/sql/execinfrapb/processors_base.proto line 45 at r1 (raw file):
// population of the DistSQL diagrams, and if set, it takes precedence over // OutputColumns. repeated uint32 original_output_columns = 7 [packed = true];
nit: It looks like these fields were added in #71441 which I think was in 22.1 (is that right?). Is it a problem to remove these fields in 22.2? During an unfinalized upgrade from 22.1 to 22.2, could a 22.2 node receive a PostProcessSpec from a 22.1 node with these fields set?
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/execinfrapb/processors_base.proto line 45 at r1 (raw file):
is that right?
Yep, these fields were added in 22.1 release cycle, and we stopped using them also during 22.1 cycle (if I'm not mistaken) 😄
Is it a problem to remove these fields in 22.2?
No. Note that even though we're removing the fields, we're marking them as reserved which is backwards-compatible way of removing protobuf fields. In a mixed 22.1-22.2 setup the old nodes will populate these fields, but then the newer nodes will ignore them; if the newer node is the gateway, then it won't populate the fields, but the older nodes will still be able to deserialize the messages (but these fields will be left unset - which is ok).
|
Build succeeded: |
sql: remove no longer used fields from PostProcessSpec
This commit removes no longer used
OriginalOutputColumnsandOriginalRenderExprsfields from thePostProcessSpec- theyare no longer needed because we now propagate the set of needed
columns in the
IndexFetchSpec.Release note: None
colbuilder: check native support via processor core rather than the spec
This commit is just a mechanical change.
Release note: None
colbuilder: vectorizing rendering on top of wrapped processors
Previously, whenever we needed to wrap a row-by-row processor into the
vectorized flow, we would pass in the whole
PostProcessSpecto thatprocessor. When this was originally implemented several years ago, this
was needed so that processors could determine the set of "needed
columns" (which should be decoded from KV responses). However, recently
we refactored that, and now the set of needed columns is passed via the
IndexFetchSpec. This means that we no longer need to pass the wholePostProcessSpecwhen wrapping, and this commit takes advantage of thatobservation in order to vectorize the evaluation of render expressions.
In particular, vectorizing of render expressions allows us to plan more
efficient vectorized builtins. We still pass all other parts of the
PostProcessSpec(meaning that the wrapped processor is stillresponsible for projections, limits, and offsets) since those operations
will limit the number of datums that need to be converted to the
columnar in-memory format as well as provide limit hints to the wrapped
processors.
Release note: None