Support offset frame type #7514

xzhangxian1008 · 2023-05-19T07:36:44Z

What problem does this PR solve?

Issue Number: ref #7376

Problem Summary:

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

…rst_val

…p_offset

ti-chi-bot · 2023-05-19T07:36:46Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
windtalker

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

xzhangxian1008 · 2023-05-19T07:36:54Z

/cc @windtalker @SeaRise

dbms/src/DataStreams/WindowBlockInputStream.cpp

dbms/src/WindowFunctions/tests/gtest_last_value.cpp

dbms/src/DataStreams/WindowBlockInputStream.cpp

Co-authored-by: SeaRise <hhssearise@foxmail.com>

dbms/src/DataStreams/WindowBlockInputStream.cpp

xzhangxian1008 · 2023-05-29T01:20:04Z

/cc @SeaRise

SeaRise

Others LGTM

dbms/src/DataStreams/WindowBlockInputStream.cpp

dbms/src/WindowFunctions/tests/gtest_last_value.cpp

dbms/src/WindowFunctions/tests/gtest_first_value.cpp

dbms/src/DataStreams/WindowBlockInputStream.cpp

xzhangxian1008 · 2023-05-29T08:17:17Z

/cc @SeaRise

dbms/src/DataStreams/WindowBlockInputStream.cpp

dbms/src/Interpreters/WindowDescription.h

windtalker · 2023-05-29T09:20:24Z

dbms/src/DataStreams/WindowBlockInputStream.cpp

@@ -296,6 +409,9 @@ void WindowTransformAction::advanceFrameStart()
        break;
    }
    case WindowFrame::BoundaryType::Offset:
+        frame_start = stepForward(current_row, window_description.frame.begin_offset);


The stepForward only works for rows frame type, need explicitly throw error if the frame type is range or groups.

The stepForward only works for rows frame type, need explicitly throw error if the frame type is range or groups.

Both rows and range will reach to here, so I think we should use the correct function according to frame type, instead of throwing exception.

Does this means you will support range type in this pr?

Does this means you will support range type in this pr?

No, but I will throw exception for range type here.

windtalker · 2023-05-29T09:20:47Z

dbms/src/DataStreams/WindowBlockInputStream.cpp

@@ -396,6 +512,10 @@ void WindowTransformAction::advanceFrameEnd()
        break;
    }
    case WindowFrame::BoundaryType::Offset:
+    {
+        std::tie(frame_end, frame_ended) = stepBackward(current_row, window_description.frame.end_offset);


xzhangxian1008 · 2023-05-31T06:14:31Z

/cc @windtalker

dbms/src/DataStreams/WindowBlockInputStream.cpp

windtalker

Others LGTM

dbms/src/DataStreams/WindowBlockInputStream.h

Lloyd-Pottiger · 2023-06-02T04:06:21Z

/merge

ti-chi-bot · 2023-06-02T04:06:23Z

@Lloyd-Pottiger: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2023-06-02T04:06:24Z

This pull request has been accepted and is ready to merge.

Commit hash: c4dbd55

xzhangxian1008 and others added 18 commits May 6, 2023 10:18

start

06fe3d6

format

5fafd74

address comments

315eaa7

tweaking

4d24724

add ft

5c3d0b0

fix ft

b8b4338

Merge branch 'master' into first_val

6b3b730

fix ft

9a5d54a

Merge branch 'first_val' of github.com:xzhangxian1008/tiflash into fi…

96b5e75

…rst_val

init

caf46f1

merge first_val

8cc51f6

merge master

2252564

add frame type test for first_value

b34c4f2

Merge branch 'master' of https://github.com/pingcap/tiflash into bd_t…

283ad3f

…p_offset

Merge branch 'master' of https://github.com/pingcap/tiflash into bd_t…

0696d20

…p_offset

need fix

b9fca2f

fix

9ec608b

remove useless code

aeb33d9

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 19, 2023

ti-chi-bot bot requested review from SeaRise and windtalker May 19, 2023 07:36

SeaRise reviewed May 22, 2023

View reviewed changes

xzhangxian1008 added 2 commits May 25, 2023 10:57

Merge branch 'master' of github.com:pingcap/tiflash into bd_tp_offset

c6672f1

init

8d42fd3

xzhangxian1008 requested a review from SeaRise May 25, 2023 03:19

SeaRise reviewed May 25, 2023

View reviewed changes

Update dbms/src/DataStreams/WindowBlockInputStream.cpp

bb521cd

Co-authored-by: SeaRise <hhssearise@foxmail.com>

SeaRise reviewed May 25, 2023

View reviewed changes

address comments

a4c1d4f

ti-chi-bot bot requested a review from SeaRise May 29, 2023 01:20

SeaRise reviewed May 29, 2023

View reviewed changes

ti-chi-bot bot requested a review from SeaRise May 29, 2023 08:17

xzhangxian1008 mentioned this pull request May 29, 2023

expression: Switch on the pushdown for first_value() and last_value pingcap/tidb#44157

Merged

12 tasks

1

66f4210

SeaRise approved these changes May 29, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 29, 2023

windtalker reviewed May 29, 2023

View reviewed changes

1

b19a22a

ti-chi-bot bot requested a review from windtalker May 31, 2023 06:14

windtalker reviewed Jun 1, 2023

View reviewed changes

dbms/src/DataStreams/WindowBlockInputStream.cpp Outdated Show resolved Hide resolved

dbms/src/DataStreams/WindowBlockInputStream.cpp Show resolved Hide resolved

xzhangxian1008 added 2 commits June 2, 2023 09:28

Merge branch 'master' of github.com:pingcap/tiflash into bd_tp_offset

6183d9f

1

02a08d4

windtalker approved these changes Jun 2, 2023

View reviewed changes

dbms/src/DataStreams/WindowBlockInputStream.h Outdated Show resolved Hide resolved

dbms/src/DataStreams/WindowBlockInputStream.h Outdated Show resolved Hide resolved

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 2, 2023

rc

c4dbd55

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 2, 2023

Merge branch 'master' into bd_tp_offset

08088cd

ti-chi-bot bot merged commit cae66f2 into pingcap:master Jun 2, 2023

xzhangxian1008 deleted the bd_tp_offset branch June 2, 2023 05:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support offset frame type #7514

Support offset frame type #7514

xzhangxian1008 commented May 19, 2023

ti-chi-bot bot commented May 19, 2023 •

edited

Loading

xzhangxian1008 commented May 19, 2023

xzhangxian1008 commented May 29, 2023

SeaRise left a comment

xzhangxian1008 commented May 29, 2023

windtalker May 29, 2023

xzhangxian1008 May 30, 2023

windtalker May 30, 2023

xzhangxian1008 May 30, 2023

windtalker May 29, 2023

xzhangxian1008 commented May 31, 2023

windtalker left a comment

Lloyd-Pottiger commented Jun 2, 2023

ti-chi-bot bot commented Jun 2, 2023

ti-chi-bot bot commented Jun 2, 2023

Support offset frame type #7514

Support offset frame type #7514

Conversation

xzhangxian1008 commented May 19, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented May 19, 2023 • edited Loading

xzhangxian1008 commented May 19, 2023

xzhangxian1008 commented May 29, 2023

SeaRise left a comment

Choose a reason for hiding this comment

xzhangxian1008 commented May 29, 2023

windtalker May 29, 2023

Choose a reason for hiding this comment

xzhangxian1008 May 30, 2023

Choose a reason for hiding this comment

windtalker May 30, 2023

Choose a reason for hiding this comment

xzhangxian1008 May 30, 2023

Choose a reason for hiding this comment

windtalker May 29, 2023

Choose a reason for hiding this comment

xzhangxian1008 commented May 31, 2023

windtalker left a comment

Choose a reason for hiding this comment

Lloyd-Pottiger commented Jun 2, 2023

ti-chi-bot bot commented Jun 2, 2023

ti-chi-bot bot commented Jun 2, 2023

ti-chi-bot bot commented May 19, 2023 •

edited

Loading