Skip to content

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Jun 2, 2023

we do not need to canonicalize row-by-row when v1 and v2 schema exactly matches (and requires no conversion).

The listed pass-through types are also handled in DataBlockBuilder thus it is safe to ignore the canonicalization step in leaf-stage operator; however it doesn't mean the rest of the types requires a conversion

TODO:

  1. verify or optimize type-conversion logic.
  2. create alternative solution for type conversion required column data types:
    • BOOL (which stored as INT)
    • TIMESTAMP (which stored as LONG)
    • ... (any other types verified)
  3. possibly create the logic before reaching the leaf-stage such as doing this in COMBINE or even before COMBINE

@walterddr walterddr added performance multi-stage Related to the multi-stage query engine labels Jun 2, 2023
@walterddr walterddr force-pushed the skip_leaf_stage_canonicalization branch from b6a5742 to 451a50a Compare June 2, 2023 00:14
@codecov-commenter
Copy link

Codecov Report

Merging #10831 (451a50a) into master (6dbd9e2) will decrease coverage by 10.53%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             master   #10831       +/-   ##
=============================================
- Coverage     34.25%   23.72%   -10.53%     
+ Complexity      462       58      -404     
=============================================
  Files          2170     2154       -16     
  Lines        116650   116218      -432     
  Branches      17654    17597       -57     
=============================================
- Hits          39959    27577    -12382     
- Misses        73201    85791    +12590     
+ Partials       3490     2850      -640     
Flag Coverage Δ
integration1 ?
integration2 23.72% <0.00%> (+0.14%) ⬆️
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ot/core/operator/blocks/InstanceResponseBlock.java 67.34% <ø> (-4.09%) ⬇️
...e/operator/LeafStageTransferableBlockOperator.java 0.00% <0.00%> (ø)

... and 474 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@walterddr
Copy link
Contributor Author

closing this PR doesn't seem to make the most bang on the buck

@walterddr walterddr closed this Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-stage Related to the multi-stage query engine performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants