You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/07 02:24:44 UTC

[GitHub] [incubator-doris] luwei16 opened a new pull request, #8877: [feature](function) Support nth value window function on vectorized e…

luwei16 opened a new pull request, #8877:
URL: https://github.com/apache/incubator-doris/pull/8877

   …ngine
   
   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem Summary:
   
   Describe the overview of changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   2. Has unit tests been added: (Yes/No/No Need)
   3. Has document been added or modified: (Yes/No/No Need)
   4. Does it need to update dependencies: (Yes/No)
   5. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] commented on pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8877:
URL: https://github.com/apache/doris/pull/8877#issuecomment-1287954675

   We're closing this PR because it hasn't been updated in a while.
   This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] zhangstar333 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
zhangstar333 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r847915589


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -2336,6 +2336,10 @@ private void initAggregateBuiltins() {
                     prefix + OFFSET_FN_UPDATE_SYMBOL.get(t),
                     null, null, null, true));
 
+            addBuiltin(AggregateFunction.createAnalyticBuiltin(
+                     "nth_value", Lists.newArrayList(t, Type.BIGINT, t), t, t,

Review Comment:
   It seems that MySQL has no default value and directly returns null



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] zhangstar333 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
zhangstar333 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r847915588


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -2336,6 +2336,10 @@ private void initAggregateBuiltins() {
                     prefix + OFFSET_FN_UPDATE_SYMBOL.get(t),
                     null, null, null, true));
 
+            addBuiltin(AggregateFunction.createAnalyticBuiltin(
+                     "nth_value", Lists.newArrayList(t, Type.BIGINT, t), t, t,

Review Comment:
   It seems that MySQL has no default value and directly returns null



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -2336,6 +2336,10 @@ private void initAggregateBuiltins() {
                     prefix + OFFSET_FN_UPDATE_SYMBOL.get(t),
                     null, null, null, true));
 
+            addBuiltin(AggregateFunction.createAnalyticBuiltin(
+                     "nth_value", Lists.newArrayList(t, Type.BIGINT, t), t, t,

Review Comment:
   It seems that MySQL has no default value and directly returns null



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] luwei16 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
luwei16 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r857581933


##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -363,6 +363,47 @@ struct WindowFunctionLastData : Data {
     static const char* name() { return "last_value"; }
 };
 
+template <typename Data>
+struct WindowFunctionNthData : Data {
+    void add_range_single_place(int64_t partition_start, int64_t partition_end, int64_t frame_start,
+                                int64_t frame_end, const IColumn** columns) {
+        int64_t offset = columns[1]->get64(partition_start);
+        if (this->has_set_value()) {
+            return;
+        }
+
+        if (!is_set_frame_start) {
+            this->frame_start = std::max<int64_t>(frame_start, partition_start);

Review Comment:
   The frame_start parameter is not the real window start, only when reset is called, 
   frame_start is equal to the real window start, so I use this->frame_start to save the real window start value
   
   Below is an example
   
   the simple table is:
   |class | id | score| 
   1           1    80   
   1           2    90    
   1           3    70   
   1           4    95
   
   the sql is:
   select class_id, student_id, score, nth_value(id, 3) over (partition by class order by score desc) as 'nth' from table
   
   The parameters for add_range_single_place to run four times are:
   
   partition_start partition_end frame_start frame_end
   0                             4                       0                1             
   0                             4                       1                 2                 
   0                             4                       2                3 
   0                             4                       3                4



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] closed pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #8877: [feature](function) Support nth value window function on vectorized e…
URL: https://github.com/apache/doris/pull/8877


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] luwei16 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
luwei16 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r849180834


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -2336,6 +2336,10 @@ private void initAggregateBuiltins() {
                     prefix + OFFSET_FN_UPDATE_SYMBOL.get(t),
                     null, null, null, true));
 
+            addBuiltin(AggregateFunction.createAnalyticBuiltin(
+                     "nth_value", Lists.newArrayList(t, Type.BIGINT, t), t, t,

Review Comment:
   > It seems that MySQL has no default value and directly returns null
   
   Default value argument have been removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] luwei16 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
luwei16 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r857558808


##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -363,6 +363,47 @@ struct WindowFunctionLastData : Data {
     static const char* name() { return "last_value"; }
 };
 
+template <typename Data>
+struct WindowFunctionNthData : Data {
+    void add_range_single_place(int64_t partition_start, int64_t partition_end, int64_t frame_start,
+                                int64_t frame_end, const IColumn** columns) {
+        int64_t offset = columns[1]->get64(partition_start);
+        if (this->has_set_value()) {
+            return;
+        }
+
+        if (!is_set_frame_start) {
+            this->frame_start = std::max<int64_t>(frame_start, partition_start);
+            is_set_frame_start = true;
+        }
+        frame_end = std::min<int64_t>(frame_end, partition_end);
+
+        if (this->frame_start + offset > frame_end) {
+            this->set_is_null();

Review Comment:
   > Maybe this is the case: window is above partition_start or under partition_end, should be null also
   
   this->frame_start = std::max<int64_t>(frame_start, partition_start);
   frame_end = std::min<int64_t>(frame_end, partition_end); 
   
   Because of these two statements, the window must be inside the partition, so the above case should be no problem



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] luwei16 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
luwei16 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r849179802


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java:
##########
@@ -2336,6 +2336,10 @@ private void initAggregateBuiltins() {
                     prefix + OFFSET_FN_UPDATE_SYMBOL.get(t),
                     null, null, null, true));
 
+            addBuiltin(AggregateFunction.createAnalyticBuiltin(
+                     "nth_value", Lists.newArrayList(t, Type.BIGINT, t), t, t,

Review Comment:
   Default value have been removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] zhangstar333 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
zhangstar333 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r850358555


##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -363,6 +363,47 @@ struct WindowFunctionLastData : Data {
     static const char* name() { return "last_value"; }
 };
 
+template <typename Data>
+struct WindowFunctionNthData : Data {
+    void add_range_single_place(int64_t partition_start, int64_t partition_end, int64_t frame_start,
+                                int64_t frame_end, const IColumn** columns) {
+        int64_t offset = columns[1]->get64(partition_start);
+        if (this->has_set_value()) {
+            return;
+        }
+
+        if (!is_set_frame_start) {
+            this->frame_start = std::max<int64_t>(frame_start, partition_start);
+            is_set_frame_start = true;
+        }
+        frame_end = std::min<int64_t>(frame_end, partition_end);
+
+        if (this->frame_start + offset > frame_end) {
+            this->set_is_null();

Review Comment:
   Maybe this is the case:  window is above partition_start or under partition_end, should be null also



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] zhangstar333 commented on a diff in pull request #8877: [feature](function) Support nth value window function on vectorized e…

Posted by GitBox <gi...@apache.org>.
zhangstar333 commented on code in PR #8877:
URL: https://github.com/apache/incubator-doris/pull/8877#discussion_r850359554


##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -363,6 +363,47 @@ struct WindowFunctionLastData : Data {
     static const char* name() { return "last_value"; }
 };
 
+template <typename Data>
+struct WindowFunctionNthData : Data {
+    void add_range_single_place(int64_t partition_start, int64_t partition_end, int64_t frame_start,
+                                int64_t frame_end, const IColumn** columns) {
+        int64_t offset = columns[1]->get64(partition_start);
+        if (this->has_set_value()) {
+            return;
+        }
+
+        if (!is_set_frame_start) {
+            this->frame_start = std::max<int64_t>(frame_start, partition_start);

Review Comment:
    A little confused about this judgment is used to?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org