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

fix(#1587): pure history rows range window iterator initialize position wrongly #1739

Conversation

aceforeverd
Copy link
Collaborator

TL;DR

InnerRangeList should call SeekToFirst on construct, close #1587

previous code use root_->SeekToFirst, which result root_ to the
current row instead the first element inside window. That is different
when the window is a pure history window

…truct

previous code use `root_->SeekToFirst`, which result `root_` to the
current row instead the first element inside window. That is different
when the window is a pure history window
@aceforeverd aceforeverd added this to the v0.5 milestone Apr 29, 2022
@github-actions github-actions bot added the execute-engine hybridse sql engine label Apr 29, 2022
@github-actions
Copy link
Contributor

SDK Test Report

  73 files    73 suites   6m 20s ⏱️
174 tests 171 ✔️ 3 💤 0
215 runs  212 ✔️ 3 💤 0

Results for commit ae550eb.

@aceforeverd
Copy link
Collaborator Author

High-priority Bugs

@github-actions
Copy link
Contributor

Linux Test Report

       57 files       186 suites   1h 4m 59s ⏱️
  8 427 tests   8 419 ✔️ 8 💤 0
12 358 runs  12 350 ✔️ 8 💤 0

Results for commit ae550eb.

@github-actions
Copy link
Contributor

HybridSE Linux Test Report

       66 files       234 suites   7m 13s ⏱️
18 927 tests 18 925 ✔️ 2 💤 0

Results for commit ae550eb.

@github-actions
Copy link
Contributor

HybridSE Mac Test Report

       66 files       234 suites   6m 31s ⏱️
18 927 tests 18 925 ✔️ 2 💤 0

Results for commit ae550eb.

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #1739 (ae550eb) into main (efdb71e) will decrease coverage by 0.02%.
The diff coverage is 80.00%.

@@             Coverage Diff              @@
##               main    #1739      +/-   ##
============================================
- Coverage     73.58%   73.55%   -0.03%     
  Complexity      347      347              
============================================
  Files           613      613              
  Lines        119851   119851              
  Branches       1010     1010              
============================================
- Hits          88187    88162      -25     
- Misses        31455    31480      +25     
  Partials        209      209              
Impacted Files Coverage Δ
...ridse/src/udf/default_defs/window_functions_def.cc 78.26% <0.00%> (ø)
hybridse/include/codec/list_iterator_codec.h 80.74% <100.00%> (ø)
hybridse/include/vm/mem_catalog.h 72.53% <100.00%> (ø)
hybridse/src/vm/mem_catalog.cc 73.66% <100.00%> (ø)
...s/query_response_time/deploy_query_response_time.h 81.25% <0.00%> (-18.75%) ⬇️
...atistics/query_response_time/query_response_time.h 78.26% <0.00%> (-4.35%) ⬇️
src/sdk/db_sdk.cc 62.13% <0.00%> (-3.86%) ⬇️
.../query_response_time/deploy_query_response_time.cc 97.10% <0.00%> (-2.90%) ⬇️
src/zk/dist_lock.cc 81.81% <0.00%> (-1.52%) ⬇️
hybridse/src/udf/udf_library.cc 81.46% <0.00%> (-0.44%) ⬇️
... and 7 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 6783625...ae550eb. Read the comment docs.

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 d3067e4 into 4paradigm:main May 5, 2022
@aceforeverd aceforeverd deleted the feat/1587-pure-history-rows-range-window branch May 5, 2022 11:17
@lumianph lumianph mentioned this pull request May 13, 2022
39 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execute-engine hybridse sql engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

at/lag/first_value udf function over merged window report different result compared to those not merged
3 participants