Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support exclude current_row #2053

Merged
merged 20 commits into from
Jul 6, 2022

Conversation

aceforeverd
Copy link
Collaborator

@aceforeverd aceforeverd commented Jul 1, 2022

close #1387

Here's the overviews

  • Overview
    • window ( ROWS_RANGE .. [ 0s preceding | current row ] exclude current_row) -> special handling
      • actually a mixed type window: frame start with range offset, and frame end as rows offset
    • window (ROWS .. [ 0 preceding | current row ] exclude current_row) -> window (ROWS .. 0 open preceding)
    • lag over ( .. exclude current_row) -> no effect
    • other case -> no effect
  • plan nodes
    new field exclude_current_row_ added to those nodes
    • logical nodes
      • FrameNode -> set only by physical transformer
      • WindowDefNode -> set by planner
      • WindowPlanNode
    • physical nodes
      • PhysicalWindowAggrerationNode
      • PhysicalRequestUnionNode
  • runner
    • WindowAggRunner batch mode
    • RequestUnionNode (batch-)request mode
  • codegen
    • InnerRowsRangeList -> for (batch-)request mode, construct the InnerRowsRangeList(1, rows_end) for rows_range ( .. os preceding exclude current_row)
  • iterator
    • the HistoryWindow table handler for batch mode.

@github-actions github-actions bot added build openmldb compiling and installing docker openmldb compile image or demo image execute-engine hybridse sql engine labels Jul 1, 2022
@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #2053 (8165e8b) into main (3cc6a64) will increase coverage by 0.03%.
The diff coverage is 88.73%.

@@             Coverage Diff              @@
##               main    #2053      +/-   ##
============================================
+ Coverage     75.67%   75.71%   +0.03%     
  Complexity      347      347              
============================================
  Files           615      615              
  Lines        116901   117026     +125     
  Branches       1015     1025      +10     
============================================
+ Hits          88463    88602     +139     
+ Misses        28229    28215      -14     
  Partials        209      209              
Impacted Files Coverage Δ
hybridse/include/case/sql_case.h 94.59% <ø> (ø)
hybridse/include/codec/type_codec.h 59.82% <ø> (ø)
hybridse/include/node/node_manager.h 100.00% <ø> (ø)
hybridse/src/codegen/window_ir_builder.h 100.00% <ø> (ø)
hybridse/src/node/plan_node_test.cc 100.00% <ø> (ø)
hybridse/src/vm/core_api.cc 16.14% <0.00%> (-0.09%) ⬇️
hybridse/src/vm/core_api.h 0.00% <ø> (ø)
...idse/src/passes/physical/batch_request_optimize.cc 90.37% <25.00%> (-0.25%) ⬇️
...digm/openmldb/batch/window/WindowAggPlanUtil.scala 61.40% <50.00%> (-0.21%) ⬇️
hybridse/include/codec/list_iterator_codec.h 80.55% <80.00%> (-0.30%) ⬇️
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cc6a64...8165e8b. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

HybridSE Linux Test Report

19 278 tests   19 276 ✔️  5m 22s ⏱️
     237 suites           2 💤
       67 files             0

Results for commit 8165e8b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

HybridSE Mac Test Report

19 278 tests   19 276 ✔️  6m 56s ⏱️
     237 suites           2 💤
       67 files             0

Results for commit 8165e8b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

Linux Test Report

       54 files       199 suites   52m 32s ⏱️
  9 208 tests   9 204 ✔️ 4 💤 0
13 559 runs  13 555 ✔️ 4 💤 0

Results for commit 8165e8b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2022

SDK Test Report

  75 files    75 suites   5m 50s ⏱️
165 tests 163 ✔️ 2 💤 0
206 runs  204 ✔️ 2 💤 0

Results for commit 8165e8b.

♻️ This comment has been updated with latest results.

@aceforeverd
Copy link
Collaborator Author

exclude current row

@aceforeverd aceforeverd requested review from vagetablechicken and zhanghaohit and removed request for vagetablechicken July 4, 2022 04:23
@github-actions github-actions bot added the storage-engine openmldb storage engine. nameserver & tablet label Jul 4, 2022
when `EXCLUDE CURRENT_ROW` set, only specific window results different execute logic:
- ROWS_RANGE window where end frame bound is `0s` or `current row`

for others:
- ROWS window where end frame bound is `0 or CURRENT ROW`: just same as
  `0 OPEN PRECEDING`

- do not apply to `lag/lead/at` function.
- do not matter if window is pure history (window bound already excluded
  current row)

This is a basic support, need more tests
RequestUnionRunner should always include the current row
with combination of:
1. maxsize
2. open preceding bound
sql_node_test & planner_v2_test
- codegen a InnerRowsRangeList for Rows_ragnge exclude current_row
  window in (batch-)request mode.
- redesign the nodes supporting `exclude_current_row`
@aceforeverd aceforeverd force-pushed the feat-exclude-current-row-1387 branch from 68ed6d8 to c9c7d48 Compare July 4, 2022 06:14
@aceforeverd
Copy link
Collaborator Author

aceforeverd commented Jul 5, 2022

TODO:

  • pre-agg
  • openmldb-batch

@aceforeverd aceforeverd requested a review from tobegit3hub July 5, 2022 04:03
@github-actions github-actions bot added the batch-engine openmldb batch(offline) engine label Jul 5, 2022
Copy link
Collaborator

@tobegit3hub tobegit3hub left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@zhanghaohit zhanghaohit left a comment

Choose a reason for hiding this comment

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

LGTM

@aceforeverd aceforeverd merged commit bdfd008 into 4paradigm:main Jul 6, 2022
@aceforeverd aceforeverd deleted the feat-exclude-current-row-1387 branch July 6, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch-engine openmldb batch(offline) engine build openmldb compiling and installing docker openmldb compile image or demo image execute-engine hybridse sql engine storage-engine openmldb storage engine. nameserver & tablet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude the current row in window aggregation
4 participants