zabetak commented on a change in pull request #3011:
URL: https://github.com/apache/hive/pull/3011#discussion_r804583395



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -2026,6 +2038,10 @@ protected RelNode 
applyMaterializedViewRewriting(RelOptPlanner planner, RelNode
 
       // Optimize plan
       if (!ruleExclusionRegex.isEmpty()) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("The CBO rules matching the following regex are excluded 
from planning: {}",
+              ruleExclusionRegex);
+        }

Review comment:
       I think these logs are slightly redundant if 
`logger.CalcitePlanner.level` is set to DEBUG 
(https://github.com/apache/calcite/blob/812e3e98eae518cf85cd1b6b7f055fb96784a423/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java#L316).
 This is a bit higher level so not exactly the same thing so don't feel 
strongly about having it or not.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -2450,6 +2466,10 @@ protected RelNode executeProgram(RelNode basePlan, 
HepProgram program,
       }
 
       if (!ruleExclusionRegex.isEmpty()) {
+        if (LOG.isDebugEnabled()) {

Review comment:
       Idem as above

##########
File path: ql/src/test/results/clientpositive/llap/excluded_rule_explain.q.out
##########
@@ -0,0 +1,112 @@
+PREHOOK: query: EXPLAIN CBO SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN CBO SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+CBO PLAN:
+HiveProject(_o__c0=[1])
+  HiveTableScan(table=[[_dummy_database, _dummy_table]], 
table:alias=[_dummy_table])
+
+PREHOOK: query: EXPLAIN FORMATTED CBO SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN FORMATTED CBO SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+{"CBOPlan":"{\n  \"rels\": [\n    {\n      \"id\": \"0\",\n      \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveTableScan\",\n   
   \"table\": [\n        \"_dummy_database\",\n        \"_dummy_table\"\n      
],\n      \"table:alias\": \"_dummy_table\",\n      \"inputs\": [],\n      
\"rowCount\": 1.0,\n      \"avgRowSize\": 129.0,\n      \"rowType\": [\n        
{\n          \"type\": \"BIGINT\",\n          \"nullable\": true,\n          
\"name\": \"BLOCK__OFFSET__INSIDE__FILE\"\n        },\n        {\n          
\"type\": \"VARCHAR\",\n          \"nullable\": true,\n          \"precision\": 
2147483647,\n          \"name\": \"INPUT__FILE__NAME\"\n        },\n        {\n 
         \"fields\": [\n            {\n              \"type\": \"BIGINT\",\n    
          \"nullable\": true,\n              \"name\": \"writeid\"\n            
},\n            {\n              \"type\": \"INTEGER\",\n              
\"nullable\": true,\n              \"name\": \"bucketid\"\n    
         },\n            {\n              \"type\": \"BIGINT\",\n              
\"nullable\": true,\n              \"name\": \"rowid\"\n            }\n         
 ],\n          \"name\": \"ROW__ID\"\n        },\n        {\n          
\"type\": \"BOOLEAN\",\n          \"nullable\": true,\n          \"name\": 
\"ROW__IS__DELETED\"\n        }\n      ]\n    },\n    {\n      \"id\": \"1\",\n 
     \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveProject\",\n     
 \"fields\": [\n        \"_o__c0\"\n      ],\n      \"exprs\": [\n        {\n   
       \"literal\": 1,\n          \"type\": {\n            \"type\": 
\"INTEGER\",\n            \"nullable\": false\n          }\n        }\n      
],\n      \"rowCount\": 1.0\n    }\n  ]\n}"}
+PREHOOK: query: EXPLAIN SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+STAGE DEPENDENCIES:
+  Stage-0 is a root stage
+
+STAGE PLANS:
+  Stage: Stage-0
+    Fetch Operator
+      limit: -1
+      Processor Tree:
+        TableScan
+          alias: _dummy_table
+          Row Limit Per Split: 1
+          Select Operator
+            expressions: 1 (type: int)
+            outputColumnNames: _col0
+            ListSink
+
+PREHOOK: query: EXPLAIN FORMATTED SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN FORMATTED SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+{"CBOPlan":"{\n  \"rels\": [\n    {\n      \"id\": \"0\",\n      \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveTableScan\",\n   
   \"table\": [\n        \"_dummy_database\",\n        \"_dummy_table\"\n      
],\n      \"table:alias\": \"_dummy_table\",\n      \"inputs\": [],\n      
\"rowCount\": 1.0,\n      \"avgRowSize\": 0.0,\n      \"rowType\": [\n        
{\n          \"type\": \"BIGINT\",\n          \"nullable\": true,\n          
\"name\": \"BLOCK__OFFSET__INSIDE__FILE\"\n        },\n        {\n          
\"type\": \"VARCHAR\",\n          \"nullable\": true,\n          \"precision\": 
2147483647,\n          \"name\": \"INPUT__FILE__NAME\"\n        },\n        {\n 
         \"fields\": [\n            {\n              \"type\": \"BIGINT\",\n    
          \"nullable\": true,\n              \"name\": \"writeid\"\n            
},\n            {\n              \"type\": \"INTEGER\",\n              
\"nullable\": true,\n              \"name\": \"bucketid\"\n      
       },\n            {\n              \"type\": \"BIGINT\",\n              
\"nullable\": true,\n              \"name\": \"rowid\"\n            }\n         
 ],\n          \"name\": \"ROW__ID\"\n        },\n        {\n          
\"type\": \"BOOLEAN\",\n          \"nullable\": true,\n          \"name\": 
\"ROW__IS__DELETED\"\n        }\n      ]\n    },\n    {\n      \"id\": \"1\",\n 
     \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveProject\",\n     
 \"fields\": [\n        \"_o__c0\"\n      ],\n      \"exprs\": [\n        {\n   
       \"literal\": 1,\n          \"type\": {\n            \"type\": 
\"INTEGER\",\n            \"nullable\": false\n          }\n        }\n      
],\n      \"rowCount\": 1.0\n    }\n  ]\n}","optimizedSQL":"SELECT 1 AS 
`_o__c0`\nFROM `_dummy_database`.`_dummy_table`","cboInfo":"Plan optimized by 
CBO.","STAGE DEPENDENCIES":{"Stage-0":{"ROOT STAGE":"TRUE"}},"STAGE 
PLANS":{"Stage-0":{"Fetch Operator":{"limit:":"-1","Processor Tree:":{"Tabl
 eScan":{"alias:":"_dummy_table","database:":"_dummy_database","Row Limit Per 
Split:":"1","table:":"_dummy_table","isTempTable:":"false","OperatorId:":"TS_0","children":{"Select
 Operator":{"expressions:":"1 (type: 
int)","columnExprMap:":{"_col0":"1"},"outputColumnNames:":["_col0"],"OperatorId:":"SEL_1","children":{"ListSink":{"OperatorId:":"LIST_SINK_3"}}}}}}}}}}
+PREHOOK: query: EXPLAIN CBO SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN CBO SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+Excluded rules: HiveJoinPushTransitivePredicatesRule|HivePreFilteringRule
+
+CBO PLAN:
+HiveProject(_o__c0=[1])
+  HiveTableScan(table=[[_dummy_database, _dummy_table]], 
table:alias=[_dummy_table])
+
+PREHOOK: query: EXPLAIN FORMATTED CBO SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN FORMATTED CBO SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+{"CBOPlan":"{\n  \"rels\": [\n    {\n      \"id\": \"0\",\n      \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveTableScan\",\n   
   \"table\": [\n        \"_dummy_database\",\n        \"_dummy_table\"\n      
],\n      \"table:alias\": \"_dummy_table\",\n      \"inputs\": [],\n      
\"rowCount\": 1.0,\n      \"avgRowSize\": 129.0,\n      \"rowType\": [\n        
{\n          \"type\": \"BIGINT\",\n          \"nullable\": true,\n          
\"name\": \"BLOCK__OFFSET__INSIDE__FILE\"\n        },\n        {\n          
\"type\": \"VARCHAR\",\n          \"nullable\": true,\n          \"precision\": 
2147483647,\n          \"name\": \"INPUT__FILE__NAME\"\n        },\n        {\n 
         \"fields\": [\n            {\n              \"type\": \"BIGINT\",\n    
          \"nullable\": true,\n              \"name\": \"writeid\"\n            
},\n            {\n              \"type\": \"INTEGER\",\n              
\"nullable\": true,\n              \"name\": \"bucketid\"\n    
         },\n            {\n              \"type\": \"BIGINT\",\n              
\"nullable\": true,\n              \"name\": \"rowid\"\n            }\n         
 ],\n          \"name\": \"ROW__ID\"\n        },\n        {\n          
\"type\": \"BOOLEAN\",\n          \"nullable\": true,\n          \"name\": 
\"ROW__IS__DELETED\"\n        }\n      ]\n    },\n    {\n      \"id\": \"1\",\n 
     \"relOp\": 
\"org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveProject\",\n     
 \"fields\": [\n        \"_o__c0\"\n      ],\n      \"exprs\": [\n        {\n   
       \"literal\": 1,\n          \"type\": {\n            \"type\": 
\"INTEGER\",\n            \"nullable\": false\n          }\n        }\n      
],\n      \"rowCount\": 1.0\n    }\n  ]\n}","cboInfo":"Excluded rules: 
HiveJoinPushTransitivePredicatesRule|HivePreFilteringRule"}
+PREHOOK: query: EXPLAIN SELECT 1
+PREHOOK: type: QUERY
+PREHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+POSTHOOK: query: EXPLAIN SELECT 1
+POSTHOOK: type: QUERY
+POSTHOOK: Input: _dummy_database@_dummy_table
+#### A masked pattern was here ####
+Excluded rules: HiveJoinPushTransitivePredicatesRule|HivePreFilteringRule

Review comment:
       Shouldn't I see somewhere the "Plan optimized by CBO." statement? I 
think it appears in some other `.q.out` files.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -2026,6 +2038,10 @@ protected RelNode 
applyMaterializedViewRewriting(RelOptPlanner planner, RelNode
 
       // Optimize plan
       if (!ruleExclusionRegex.isEmpty()) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("The CBO rules matching the following regex are excluded 
from planning: {}",
+              ruleExclusionRegex);
+        }

Review comment:
       Also I don't think you need the `LOG.isDebugEnabled()` guard. There 
doesn't to seem any performance critical operation below and I know there was 
some effort to eliminate this pattern from the project (if possible).

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java
##########
@@ -64,6 +64,7 @@
 import org.apache.hadoop.hive.ql.optimizer.physical.StageIDsRearranger;
 import org.apache.hadoop.hive.ql.parse.ASTNode;
 import org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.CalcitePlanner;

Review comment:
       Minor but I don't think its worth coupling this with CalcitePlanner for 
just a prefix that we don't really care if it ends up being different.

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -575,7 +578,12 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) 
throws SemanticExcept
               getQB().getParseInfo().setHintList(oldHints);
             }
             LOG.info("CBO Succeeded; optimized logical plan.");
-            this.ctx.setCboInfo("Plan optimized by CBO.");
+
+            String cboInfo = "Plan optimized by CBO.";
+            if (!ruleExclusionRegex.isEmpty()) {
+              cboInfo = cboInfo + (" " + EXCLUDED_RULES_PREFIX + 
ruleExclusionRegex);
+            }
+            this.ctx.setCboInfo(cboInfo);

Review comment:
       Refactor the duplicate fragment somewhere?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to