starocean999 commented on code in PR #49918:
URL: https://github.com/apache/doris/pull/49918#discussion_r2053282430


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -3702,82 +3706,101 @@ private List<List<String>> 
getTableList(List<MultipartIdentifierContext> ctx) {
         return tableList;
     }
 
-    private LogicalPlan withSelectHint(LogicalPlan logicalPlan, 
List<ParserRuleContext> hintContexts) {
-        if (hintContexts.isEmpty()) {
+    private LogicalPlan withHints(LogicalPlan logicalPlan, 
List<ParserRuleContext> selectHintContexts,
+            List<ParserRuleContext> commonHintContexts) {
+        if (selectHintContexts.isEmpty() && commonHintContexts.isEmpty()) {
             return logicalPlan;
         }
-        ImmutableList.Builder<SelectHint> hints = ImmutableList.builder();
-        for (ParserRuleContext hintContext : hintContexts) {
-            SelectHintContext selectHintContext = (SelectHintContext) 
hintContext;
-            for (HintStatementContext hintStatement : 
selectHintContext.hintStatements) {
-                if (hintStatement.USE_MV() != null) {
-                    hints.add(new SelectHintUseMv("USE_MV", 
getTableList(hintStatement.tableList), true));
-                    continue;
-                } else if (hintStatement.NO_USE_MV() != null) {
-                    hints.add(new SelectHintUseMv("NO_USE_MV", 
getTableList(hintStatement.tableList), false));
-                    continue;
-                }
-                String hintName = 
hintStatement.hintName.getText().toLowerCase(Locale.ROOT);
-                switch (hintName) {
-                    case "set_var":
-                        Map<String, Optional<String>> parameters = 
Maps.newLinkedHashMap();
-                        for (HintAssignmentContext kv : 
hintStatement.parameters) {
-                            if (kv.key != null) {
-                                String parameterName = 
visitIdentifierOrText(kv.key);
-                                Optional<String> value = Optional.empty();
-                                if (kv.constantValue != null) {
-                                    Literal literal = (Literal) 
visit(kv.constantValue);
-                                    value = 
Optional.ofNullable(literal.toLegacyLiteral().getStringValue());
-                                } else if (kv.identifierValue != null) {
-                                    // maybe we should throw exception when 
the identifierValue is quoted identifier
-                                    value = 
Optional.ofNullable(kv.identifierValue.getText());
+        LogicalPlan newPlan = logicalPlan;
+        if (!selectHintContexts.isEmpty()) {
+            ImmutableList.Builder<SelectHint> hints = ImmutableList.builder();
+            for (ParserRuleContext hintContext : selectHintContexts) {
+                SelectHintContext selectHintContext = (SelectHintContext) 
hintContext;
+                for (HintStatementContext hintStatement : 
selectHintContext.hintStatements) {
+                    if (hintStatement.USE_MV() != null) {
+                        hints.add(new SelectHintUseMv("USE_MV", 
getTableList(hintStatement.tableList), true));
+                        continue;
+                    } else if (hintStatement.NO_USE_MV() != null) {
+                        hints.add(new SelectHintUseMv("NO_USE_MV", 
getTableList(hintStatement.tableList), false));
+                        continue;
+                    }
+                    String hintName = 
hintStatement.hintName.getText().toLowerCase(Locale.ROOT);
+                    switch (hintName) {
+                        case "set_var":
+                            Map<String, Optional<String>> parameters = 
Maps.newLinkedHashMap();
+                            for (HintAssignmentContext kv : 
hintStatement.parameters) {
+                                if (kv.key != null) {
+                                    String parameterName = 
visitIdentifierOrText(kv.key);
+                                    Optional<String> value = Optional.empty();
+                                    if (kv.constantValue != null) {
+                                        Literal literal = (Literal) 
visit(kv.constantValue);
+                                        value = 
Optional.ofNullable(literal.toLegacyLiteral().getStringValue());
+                                    } else if (kv.identifierValue != null) {
+                                        // maybe we should throw exception 
when the identifierValue is quoted identifier
+                                        value = 
Optional.ofNullable(kv.identifierValue.getText());
+                                    }
+                                    parameters.put(parameterName, value);
                                 }
-                                parameters.put(parameterName, value);
                             }
-                        }
-                        SelectHintSetVar setVar = new 
SelectHintSetVar(hintName, parameters);
-                        
setVar.setVarOnceInSql(ConnectContext.get().getStatementContext());
-                        hints.add(setVar);
-                        break;
-                    case "leading":
-                        List<String> leadingParameters = new ArrayList<>();
-                        for (HintAssignmentContext kv : 
hintStatement.parameters) {
-                            if (kv.key != null) {
-                                String parameterName = 
visitIdentifierOrText(kv.key);
-                                leadingParameters.add(parameterName);
+                            SelectHintSetVar setVar = new 
SelectHintSetVar(hintName, parameters);
+                            
setVar.setVarOnceInSql(ConnectContext.get().getStatementContext());
+                            hints.add(setVar);
+                            break;
+                        case "leading":
+                            List<String> leadingParameters = new ArrayList<>();
+                            for (HintAssignmentContext kv : 
hintStatement.parameters) {
+                                if (kv.key != null) {
+                                    String parameterName = 
visitIdentifierOrText(kv.key);
+                                    leadingParameters.add(parameterName);
+                                }
                             }
-                        }
-                        hints.add(new SelectHintLeading(hintName, 
leadingParameters));
-                        break;
-                    case "ordered":
-                        hints.add(new SelectHintOrdered(hintName));
-                        break;
-                    case "use_cbo_rule":
-                        List<String> useRuleParameters = new ArrayList<>();
-                        for (HintAssignmentContext kv : 
hintStatement.parameters) {
-                            if (kv.key != null) {
-                                String parameterName = 
visitIdentifierOrText(kv.key);
-                                useRuleParameters.add(parameterName);
+                            hints.add(new SelectHintLeading(hintName, 
leadingParameters));
+                            break;
+                        case "ordered":
+                            hints.add(new SelectHintOrdered(hintName));
+                            break;
+                        case "use_cbo_rule":
+                            List<String> useRuleParameters = new ArrayList<>();
+                            for (HintAssignmentContext kv : 
hintStatement.parameters) {
+                                if (kv.key != null) {
+                                    String parameterName = 
visitIdentifierOrText(kv.key);
+                                    useRuleParameters.add(parameterName);
+                                }
                             }
-                        }
-                        hints.add(new SelectHintUseCboRule(hintName, 
useRuleParameters, false));
-                        break;
-                    case "no_use_cbo_rule":
-                        List<String> noUseRuleParameters = new ArrayList<>();
-                        for (HintAssignmentContext kv : 
hintStatement.parameters) {
-                            String parameterName = 
visitIdentifierOrText(kv.key);
-                            if (kv.key != null) {
-                                noUseRuleParameters.add(parameterName);
+                            hints.add(new SelectHintUseCboRule(hintName, 
useRuleParameters, false));
+                            break;
+                        case "no_use_cbo_rule":
+                            List<String> noUseRuleParameters = new 
ArrayList<>();
+                            for (HintAssignmentContext kv : 
hintStatement.parameters) {
+                                String parameterName = 
visitIdentifierOrText(kv.key);
+                                if (kv.key != null) {
+                                    noUseRuleParameters.add(parameterName);
+                                }
                             }
+                            hints.add(new SelectHintUseCboRule(hintName, 
noUseRuleParameters, true));
+                            break;
+                        default:
+                            break;
+                    }
+                }
+            }
+            newPlan = new LogicalSelectHint<>(hints.build(), newPlan);
+        }
+        if (!commonHintContexts.isEmpty()) {
+            for (ParserRuleContext hintContext : commonHintContexts) {
+                if (hintContext instanceof SelectHintContext) {
+                    SelectHintContext commonHintContext = (SelectHintContext) 
hintContext;
+                    if (commonHintContext.hintStatement != null && 
commonHintContext.hintStatement.hintName != null) {
+                        String text = 
commonHintContext.hintStatement.hintName.getText();
+                        if (text.equalsIgnoreCase("PREAGGOPEN")) {
+                            newPlan = new LogicalCommonHint<>(newPlan);

Review Comment:
   If only support PREAGGOPEN, use LogicalPreAggOpenHint is better. Or let 
PreAggHint as member of LogicalCommonHint, it will be easier to support other 
hint type in future 



-- 
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-unsubscr...@doris.apache.org

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


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

Reply via email to