Copilot commented on code in PR #6700:
URL: https://github.com/apache/hive/pull/6700#discussion_r3780474889
##########
ql/src/test/org/apache/hadoop/hive/ql/parse/TestSemanticAnalyzer.java:
##########
@@ -545,4 +546,60 @@ private void testMaterializeCTEUsesDDLFactory(boolean
cboEnabled) throws Excepti
cteAnalyzer[0] instanceof CreateTableAnalyzer);
}
}
+
+ @Test
+ public void testMaterializedCteInputsAndColumnAccess() throws Exception {
+ createKeyValueTable("src");
+
+ HiveConf testConf = new HiveConf(conf);
+ testConf.setIntVar(HiveConf.ConfVars.HIVE_CTE_MATERIALIZE_THRESHOLD, 1);
+
testConf.setBoolVar(HiveConf.ConfVars.HIVE_CTE_MATERIALIZE_FULL_AGGREGATE_ONLY,
false);
+ testConf.setBoolVar(HiveConf.ConfVars.HIVE_STATS_COLLECT_SCANCOLS, true);
+
+ SessionState.start(testConf);
+ String[] queries = {
+ "with q1 as ( select key from q2 where key = '5'),"
+ + "q2 as ( select key from src where key = '5') "
+ + "select * from (select key from q1) a",
+ "WITH q1 AS ("
+ + "WITH q2 AS (SELECT key, value FROM src WHERE key = '4') "
+ + "SELECT * FROM q2 UNION ALL SELECT * FROM q2) "
+ + "SELECT * FROM q1 t1 JOIN q1 t2 ON t1.key = t2.key"
+ };
+
+ SemanticAnalyzer[] analyzers = new SemanticAnalyzer[queries.length];
+ for (int i = 0; i < queries.length; i++) {
+ Context ctx = new Context(testConf);
+ ASTNode astNode = ParseUtils.parse(queries[i], ctx);
+ QueryState queryState = new
QueryState.Builder().withHiveConf(testConf).build();
+ SemanticAnalyzer analyzer = (SemanticAnalyzer)
SemanticAnalyzerFactory.get(queryState, astNode);
+ analyzer.initCtx(ctx);
+ analyzer.analyze(astNode, ctx);
+ analyzers[i] = analyzer;
Review Comment:
This test analyzes queries but never calls `endAnalysis(astNode)`. Other
tests in this class call `endAnalysis` (mirrors production Compiler behavior)
to finalize analyzer state; skipping it can hide regressions around query
properties and cleanup.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessAnalyzer.java:
##########
@@ -58,6 +59,22 @@ public ColumnAccessInfo analyzeColumnAccess(ColumnAccessInfo
columnAccessInfo) t
}
}
}
+ // Every Analyzer holds its private rootClause
Review Comment:
The comment says the analyzer holds a "private rootClause", but
`SemanticAnalyzer.rootClause` was changed to package-visible. Either keep
`rootClause` private (and expose an accessor) or update this comment so it
stays accurate.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -1406,7 +1406,7 @@ private void addCTEAsSubQuery(QB qb, String cteName,
String cteAlias)
qb.rewriteCTEToSubq(cteAlias, cteName, cteQBExpr);
}
- private final CTEClause rootClause = new CTEClause(null, null, null);
+ final CTEClause rootClause = new CTEClause(null, null, null);
Review Comment:
`SemanticAnalyzer.rootClause` was widened from `private` to package-visible
so `ColumnAccessAnalyzer` can reach into it. This increases package-level
coupling and makes refactors riskier (any class in the package can now mutate
internal CTE state). Consider keeping the field `private` and adding a narrow
(package-private) accessor that returns the CTE execution-order list needed for
column-access merging.
--
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]
