This is an automated email from the ASF dual-hosted git repository. hui pushed a commit to branch lmh/RefactorAnalyzer in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 15f4c988beab045a142803cc9104fd6abe492256 Author: liuminghui233 <[email protected]> AuthorDate: Mon Sep 26 22:54:28 2022 +0800 fix LogicalPlannerTest --- .../apache/iotdb/db/mpp/plan/analyze/Analysis.java | 11 ++-- .../iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java | 61 ++++++++++++++-------- .../db/mpp/plan/analyze/ExpressionAnalyzer.java | 17 +++--- .../db/mpp/plan/planner/LogicalPlanBuilder.java | 7 ++- .../db/mpp/plan/planner/LogicalPlanVisitor.java | 8 +-- .../iotdb/db/mpp/plan/analyze/AnalyzeTest.java | 4 +- .../iotdb/db/mpp/plan/plan/LogicalPlannerTest.java | 2 + .../db/mpp/plan/plan/QueryLogicalPlanUtil.java | 4 +- 8 files changed, 63 insertions(+), 51 deletions(-) diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analysis.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analysis.java index e393044b6c..757bfa42fb 100644 --- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analysis.java +++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analysis.java @@ -107,7 +107,7 @@ public class Analysis { // e.g. [s1,s2,s3] is query, but [s1, s3] exists in device1, then device1 -> [1, 3], s1 is 1 but // not 0 because device is the first column - private Map<String, List<Integer>> deviceToMeasurementIndexesMap; + private Map<String, List<Integer>> deviceViewInputIndexesMap; private Set<Expression> deviceViewOutputExpressions; @@ -286,13 +286,12 @@ public class Analysis { this.finishQueryAfterAnalyze = finishQueryAfterAnalyze; } - public void setDeviceToMeasurementIndexesMap( - Map<String, List<Integer>> deviceToMeasurementIndexesMap) { - this.deviceToMeasurementIndexesMap = deviceToMeasurementIndexesMap; + public void setDeviceViewInputIndexesMap(Map<String, List<Integer>> deviceViewInputIndexesMap) { + this.deviceViewInputIndexesMap = deviceViewInputIndexesMap; } - public Map<String, List<Integer>> getDeviceToMeasurementIndexesMap() { - return deviceToMeasurementIndexesMap; + public Map<String, List<Integer>> getDeviceViewInputIndexesMap() { + return deviceViewInputIndexesMap; } public Set<Expression> getSourceExpressions() { diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java index 5b261eeaa9..bb7dcca3ef 100644 --- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java +++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java @@ -228,8 +228,7 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext> analyzeDeviceToSourceTransform(analysis, queryStatement); analyzeDeviceToSource(analysis, queryStatement); - - analyzeDeviceView(analysis, queryStatement, outputExpressions, new LinkedHashMap<>()); + analyzeDeviceView(analysis, queryStatement, outputExpressions); } else { // Example 1: select s1, s1 + s2 as t, udf(udf(s1)) from root.sg.d1 // outputExpressions: [<root.sg.d1.s1,null>, <root.sg.d1.s1 + root.sg.d1.s2,t>, @@ -301,7 +300,7 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext> Expression predicate = queryStatement.getWhereCondition().getPredicate(); WhereCondition whereCondition = queryStatement.getWhereCondition(); Pair<Filter, Boolean> resultPair = - ExpressionAnalyzer.transformToGlobalTimeFilter(predicate, true, true); + ExpressionAnalyzer.extractGlobalTimeFilter(predicate, true, true); predicate = ExpressionAnalyzer.evaluatePredicate(predicate); // set where condition to null if predicate is true @@ -717,17 +716,21 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext> if (queryStatement.isGroupByLevel()) { Set<Expression> aggregationExpressions = - new HashSet<>(analysis.getGroupByLevelExpressions().keySet()); + analysis.getGroupByLevelExpressions().values().stream() + .flatMap(Set::stream) + .collect(Collectors.toSet()); + analysis.setAggregationExpressions(aggregationExpressions); + } else { + Set<Expression> aggregationExpressions = new HashSet<>(); + for (Expression expression : analysis.getSelectExpressions()) { + aggregationExpressions.addAll(ExpressionAnalyzer.searchAggregationExpressions(expression)); + } + if (queryStatement.hasHaving()) { + aggregationExpressions.addAll( + ExpressionAnalyzer.searchAggregationExpressions(analysis.getHavingExpression())); + } analysis.setAggregationExpressions(aggregationExpressions); } - - Set<Expression> aggregationExpressions = new HashSet<>(); - for (Expression expression : analysis.getSelectExpressions()) { - aggregationExpressions.addAll(ExpressionAnalyzer.searchAggregationExpressions(expression)); - } - aggregationExpressions.addAll( - ExpressionAnalyzer.searchAggregationExpressions(analysis.getHavingExpression())); - analysis.setAggregationExpressions(aggregationExpressions); } private void analyzeDeviceToSourceTransform(Analysis analysis, QueryStatement queryStatement) { @@ -858,8 +861,7 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext> private void analyzeDeviceView( Analysis analysis, QueryStatement queryStatement, - List<Pair<Expression, String>> outputExpressions, - Map<String, Set<String>> deviceToMeasurementsMap) { + List<Pair<Expression, String>> outputExpressions) { Set<Expression> selectExpressions = outputExpressions.stream() .map(Pair::getLeft) @@ -873,22 +875,35 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext> } analysis.setDeviceViewOutputExpressions(deviceViewOutputExpressions); - List<String> allMeasurements = + List<String> deviceViewOutputColumns = deviceViewOutputExpressions.stream() .map(Expression::getExpressionString) .collect(Collectors.toList()); - Map<String, List<Integer>> deviceToMeasurementIndexesMap = new HashMap<>(); - for (String deviceName : deviceToMeasurementsMap.keySet()) { - List<String> measurementsUnderDevice = - new ArrayList<>(deviceToMeasurementsMap.get(deviceName)); + Map<String, Set<String>> deviceToOutputColumnsMap = new LinkedHashMap<>(); + Map<String, Set<Expression>> deviceToOutputExpressions = + queryStatement.isAggregationQuery() + ? analysis.getDeviceToAggregationExpressions() + : analysis.getDeviceToSourceTransformExpressions(); + for (String deviceName : deviceToOutputExpressions.keySet()) { + Set<Expression> outputExpressionsUnderDevice = deviceToOutputExpressions.get(deviceName); + Set<String> outputColumns = new LinkedHashSet<>(); + for (Expression expression : outputExpressionsUnderDevice) { + outputColumns.add(ExpressionAnalyzer.getMeasurementExpression(expression).toString()); + } + deviceToOutputColumnsMap.put(deviceName, outputColumns); + } + + Map<String, List<Integer>> deviceViewInputIndexesMap = new HashMap<>(); + for (String deviceName : deviceToOutputColumnsMap.keySet()) { + List<String> outputsUnderDevice = new ArrayList<>(deviceToOutputColumnsMap.get(deviceName)); List<Integer> indexes = new ArrayList<>(); - for (String measurement : measurementsUnderDevice) { - indexes.add(allMeasurements.indexOf(measurement) + 1); // add 1 to skip device column + for (String output : outputsUnderDevice) { + indexes.add(deviceViewOutputColumns.indexOf(output) + 1); // add 1 to skip device column } - deviceToMeasurementIndexesMap.put(deviceName, indexes); + deviceViewInputIndexesMap.put(deviceName, indexes); } - analysis.setDeviceToMeasurementIndexesMap(deviceToMeasurementIndexesMap); + analysis.setDeviceViewInputIndexesMap(deviceViewInputIndexesMap); } private void analyzeOutput( diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/ExpressionAnalyzer.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/ExpressionAnalyzer.java index 1879ddcf87..9996ad0f93 100644 --- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/ExpressionAnalyzer.java +++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/ExpressionAnalyzer.java @@ -785,14 +785,14 @@ public class ExpressionAnalyzer { * @param isFirstOr whether it is the first LogicOrExpression encountered * @return global time filter */ - public static Pair<Filter, Boolean> transformToGlobalTimeFilter( + public static Pair<Filter, Boolean> extractGlobalTimeFilter( Expression predicate, boolean canRewrite, boolean isFirstOr) { if (predicate.getExpressionType().equals(ExpressionType.LOGIC_AND)) { Pair<Filter, Boolean> leftResultPair = - transformToGlobalTimeFilter( + extractGlobalTimeFilter( ((BinaryExpression) predicate).getLeftExpression(), canRewrite, isFirstOr); Pair<Filter, Boolean> rightResultPair = - transformToGlobalTimeFilter( + extractGlobalTimeFilter( ((BinaryExpression) predicate).getRightExpression(), canRewrite, isFirstOr); // rewrite predicate to avoid duplicate calculation on time filter @@ -821,10 +821,9 @@ public class ExpressionAnalyzer { return new Pair<>(null, true); } else if (predicate.getExpressionType().equals(ExpressionType.LOGIC_OR)) { Pair<Filter, Boolean> leftResultPair = - transformToGlobalTimeFilter( - ((BinaryExpression) predicate).getLeftExpression(), false, false); + extractGlobalTimeFilter(((BinaryExpression) predicate).getLeftExpression(), false, false); Pair<Filter, Boolean> rightResultPair = - transformToGlobalTimeFilter( + extractGlobalTimeFilter( ((BinaryExpression) predicate).getRightExpression(), false, false); if (leftResultPair.left != null && rightResultPair.left != null) { @@ -841,7 +840,7 @@ public class ExpressionAnalyzer { return new Pair<>(null, true); } else if (predicate.getExpressionType().equals(ExpressionType.LOGIC_NOT)) { Pair<Filter, Boolean> childResultPair = - transformToGlobalTimeFilter( + extractGlobalTimeFilter( ((UnaryExpression) predicate).getExpression(), canRewrite, isFirstOr); return new Pair<>(FilterFactory.not(childResultPair.left), childResultPair.right); } else if (predicate.isCompareBinaryExpression()) { @@ -906,11 +905,9 @@ public class ExpressionAnalyzer { } /** - * Search for subexpressions that can be queried natively, including time series raw data and - * built-in aggregate functions. + * Search for subexpressions that can be queried natively, including all time series. * * @param expression expression to be searched - * @param isRawDataSource if true, built-in aggregate functions are not be returned * @return searched subexpression list */ public static List<Expression> searchSourceExpressions(Expression expression) { diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanBuilder.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanBuilder.java index 997386a4d1..6d67369d75 100644 --- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanBuilder.java +++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanBuilder.java @@ -391,10 +391,13 @@ public class LogicalPlanBuilder { Set<Expression> deviceViewOutputExpressions, Map<String, List<Integer>> deviceToMeasurementIndexesMap, Ordering mergeOrder) { - List<String> outputColumnNames = + List<String> outputColumnNames = new ArrayList<>(); + outputColumnNames.add(COLUMN_DEVICE); + outputColumnNames.addAll( deviceViewOutputExpressions.stream() .map(Expression::getExpressionString) - .collect(Collectors.toList()); + .collect(Collectors.toList())); + DeviceViewNode deviceViewNode = new DeviceViewNode( context.getQueryId().genPlanNodeId(), diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanVisitor.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanVisitor.java index 4e2f0af2d3..c45a59d50a 100644 --- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanVisitor.java +++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/LogicalPlanVisitor.java @@ -125,7 +125,7 @@ public class LogicalPlanVisitor extends StatementVisitor<PlanNode, MPPQueryConte planBuilder.planDeviceView( deviceToSubPlanMap, analysis.getDeviceViewOutputExpressions(), - analysis.getDeviceToMeasurementIndexesMap(), + analysis.getDeviceViewInputIndexesMap(), queryStatement.getResultTimeOrder()); } else { planBuilder = @@ -142,10 +142,6 @@ public class LogicalPlanVisitor extends StatementVisitor<PlanNode, MPPQueryConte // other upstream node planBuilder = planBuilder - .planGroupByLevel( - analysis.getGroupByLevelExpressions(), - analysis.getGroupByTimeParameter(), - queryStatement.getResultTimeOrder()) .planHaving( analysis.getHavingExpression(), analysis.getSelectExpressions(), @@ -185,7 +181,7 @@ public class LogicalPlanVisitor extends StatementVisitor<PlanNode, MPPQueryConte } else { // aggregation query boolean isRawDataSource = - (whereExpression != null) || needTransform(sourceTransformExpressions); + analysis.hasValueFilter() || needTransform(sourceTransformExpressions); AggregationStep curStep; if (isRawDataSource) { planBuilder = diff --git a/server/src/test/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeTest.java b/server/src/test/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeTest.java index 061ef9bb19..e1907490b0 100644 --- a/server/src/test/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeTest.java +++ b/server/src/test/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeTest.java @@ -228,8 +228,8 @@ public class AnalyzeTest { actualAnalysis.getDeviceToSelectExpressions(), expectedAnalysis.getDeviceToSelectExpressions()); assertEquals( - actualAnalysis.getDeviceToMeasurementIndexesMap(), - expectedAnalysis.getDeviceToMeasurementIndexesMap()); + actualAnalysis.getDeviceViewInputIndexesMap(), + expectedAnalysis.getDeviceViewInputIndexesMap()); assertEquals( actualAnalysis.getDeviceViewOutputExpressions(), actualAnalysis.getDeviceViewOutputExpressions()); diff --git a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java index 1646afe43f..2eba05f1f5 100644 --- a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java +++ b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java @@ -73,6 +73,8 @@ public class LogicalPlannerTest { @Test public void testQueryPlan() { for (String sql : querySQLs) { + PlanNode a = parseSQLToPlanNode(sql); + PlanNode e = sqlToPlanMap.get(sql); Assert.assertEquals(sqlToPlanMap.get(sql), parseSQLToPlanNode(sql)); System.out.printf("\"%s\" TEST PASSED\n", sql); } diff --git a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/QueryLogicalPlanUtil.java b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/QueryLogicalPlanUtil.java index 7f26a71ca0..e0e918cf6e 100644 --- a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/QueryLogicalPlanUtil.java +++ b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/QueryLogicalPlanUtil.java @@ -725,8 +725,8 @@ public class QueryLogicalPlanUtil { new TimeJoinNode(queryId.genPlanNodeId(), Ordering.DESC, sourceNodeList2); Map<String, List<Integer>> deviceToMeasurementIndexesMap = new HashMap<>(); - deviceToMeasurementIndexesMap.put("root.sg.d1", Arrays.asList(2, 1, 3)); - deviceToMeasurementIndexesMap.put("root.sg.d2", Arrays.asList(2, 1, 3)); + deviceToMeasurementIndexesMap.put("root.sg.d1", Arrays.asList(1, 2, 3)); + deviceToMeasurementIndexesMap.put("root.sg.d2", Arrays.asList(1, 2, 3)); DeviceViewNode deviceViewNode = new DeviceViewNode( queryId.genPlanNodeId(),
