From 324aa1744b96dde3e644ffbed609f47936537485 Mon Sep 17 00:00:00 2001 From: zhangstar333 <2561612514@qq.com> Date: Mon, 18 Apr 2022 11:51:49 +0800 Subject: [PATCH 1/2] [Bug]fix lag/lead function get invalid data --- .../apache/doris/analysis/AnalyticExpr.java | 12 +++--- .../org/apache/doris/catalog/FunctionSet.java | 4 +- .../data/correctness/test_lag_lead_window.out | 11 +++++ .../correctness/test_lag_lead_window.groovy | 43 +++++++++++++++++++ 4 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 regression-test/data/correctness/test_lag_lead_window.out create mode 100644 regression-test/suites/correctness/test_lag_lead_window.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java index b7e73864f5c443..d2814e2df0cd97 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java @@ -478,11 +478,13 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException { // min/max is not currently supported on sliding windows (i.e. start bound is not // unbounded). - if (window != null && isMinMax(fn) && - window.getLeftBoundary().getType() != BoundaryType.UNBOUNDED_PRECEDING) { - throw new AnalysisException( - "'" + getFnCall().toSql() + "' is only supported with an " - + "UNBOUNDED PRECEDING start bound."); + if (!VectorizedUtil.isVectorized()) { + if (window != null && isMinMax(fn) && + window.getLeftBoundary().getType() != BoundaryType.UNBOUNDED_PRECEDING) { + throw new AnalysisException( + "'" + getFnCall().toSql() + "' is only supported with an " + + "UNBOUNDED PRECEDING start bound."); + } } setChildren(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java index ac8403843ef1b4..8c84af99d62dc1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/FunctionSet.java @@ -2412,13 +2412,13 @@ private void initAggregateBuiltins() { "lag", Lists.newArrayList(t, Type.BIGINT, t), t, t, prefix + OFFSET_FN_INIT_SYMBOL.get(t), prefix + OFFSET_FN_UPDATE_SYMBOL.get(t), - null, null, null)); + null, t.isStringType() ? stringValGetValue : null, null)); addBuiltin(AggregateFunction.createAnalyticBuiltin( "lead", Lists.newArrayList(t, Type.BIGINT, t), t, t, prefix + OFFSET_FN_INIT_SYMBOL.get(t), prefix + OFFSET_FN_UPDATE_SYMBOL.get(t), - null, null, null)); + null, t.isStringType() ? stringValGetValue : null, null)); //vec addBuiltin(AggregateFunction.createAnalyticBuiltin( "lag", Lists.newArrayList(t, Type.BIGINT, t), t, t, diff --git a/regression-test/data/correctness/test_lag_lead_window.out b/regression-test/data/correctness/test_lag_lead_window.out new file mode 100644 index 00000000000000..ce241315174b2a --- /dev/null +++ b/regression-test/data/correctness/test_lag_lead_window.out @@ -0,0 +1,11 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select_default -- +/wyyt-image/2021/11/13/595345040188712460.jpg +/wyyt-image/2022/04/13/1434607674511761493.jpg +/wyyt-image/2022/04/13/1434607674511761493.jpg /wyyt-image/2022/04/13/1434607674511761493.jpg + +-- !select_default2 -- +/wyyt-image/2021/11/13/595345040188712460.jpg +/wyyt-image/2022/04/13/1434607674511761493.jpg /wyyt-image/2022/04/13/1434607674511761493.jpg +/wyyt-image/2022/04/13/1434607674511761493.jpg + diff --git a/regression-test/suites/correctness/test_lag_lead_window.groovy b/regression-test/suites/correctness/test_lag_lead_window.groovy new file mode 100644 index 00000000000000..69ad1eeab4a887 --- /dev/null +++ b/regression-test/suites/correctness/test_lag_lead_window.groovy @@ -0,0 +1,43 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_lag_lead_window") { + def tableName = "wftest" + + + sql """ DROP TABLE IF EXISTS ${tableName} """ + sql """ + CREATE TABLE ${tableName} ( `aa` varchar(10) NULL COMMENT "", `bb` text NULL COMMENT "", `cc` text NULL COMMENT "" ) + ENGINE=OLAP UNIQUE KEY(`aa`) DISTRIBUTED BY HASH(`aa`) BUCKETS 3 + PROPERTIES ( "replication_allocation" = "tag.location.default: 1", "in_memory" = "false", "storage_format" = "V2" ); + """ + + sql """ INSERT INTO ${tableName} VALUES + ('a','aa','/wyyt-image/2021/11/13/595345040188712460.jpg'), + ('b','aa','/wyyt-image/2022/04/13/1434607674511761493.jpg'), + ('c','cc','/wyyt-image/2022/04/13/1434607674511761493.jpg') """ + + // not_vectorized + sql """ set enable_vectorized_engine = false """ + + qt_select_default """ select min(t.cc) over(PARTITION by t.cc order by t.aa) , + lag(t.cc,1,'') over (PARTITION by t.cc order by t.aa) as l1 from ${tableName} t; """ + + qt_select_default2 """ select min(t.cc) over(PARTITION by t.cc order by t.aa) , + lead(t.cc,1,'') over (PARTITION by t.cc order by t.aa) as l1 from ${tableName} t; """ + +} \ No newline at end of file From bb6b6cce89d1e47c38564c4a488cf6d81eedaaa5 Mon Sep 17 00:00:00 2001 From: zhangstar333 <2561612514@qq.com> Date: Mon, 18 Apr 2022 15:01:24 +0800 Subject: [PATCH 2/2] add some comment info --- .../main/java/org/apache/doris/analysis/AnalyticExpr.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java index d2814e2df0cd97..7eae49924f1009 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyticExpr.java @@ -476,9 +476,11 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException { standardize(analyzer); - // min/max is not currently supported on sliding windows (i.e. start bound is not - // unbounded). + // But in Vectorized mode, after calculate a window, will be call reset() to reset state, + // And then restarted calculate next new window; if (!VectorizedUtil.isVectorized()) { + // min/max is not currently supported on sliding windows (i.e. start bound is not + // unbounded). if (window != null && isMinMax(fn) && window.getLeftBoundary().getType() != BoundaryType.UNBOUNDED_PRECEDING) { throw new AnalysisException(