abstractdog commented on code in PR #5613:
URL: https://github.com/apache/hive/pull/5613#discussion_r1939750692
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -2094,4 +2117,79 @@ protected String getFullTableNameForSQL(ASTNode n)
throws SemanticException {
}
}
+ public QB getQB(){
+ return null;
+ }
+
+ /**
+ * Resolves the query type from the SemanticAnalyzer and the query AST,
which might look magic.
+ * Magic is needed because a query can fail due to semantic exceptions, and
at that time, from syntax point of view,
+ * it's already clear what's the query type, even if the SemanticAnalyzer
bailed out, typical problems:
+ * 1. general semantic exception
+ * 2. transaction manager validation (throwin exception)
+ * What a user expects here is something like "QUERY", "DDL", "DML", so this
magic will do its best to tell.
+ * any kind of "analyze": STATS
+ * DML operations (INSERT, UPDATE, DELETE, MERGE): DML
+ * MAPRED: QUERY, DML (depending on QueryProperties achieved in compile time)
+ * FETCH: QUERY, as a simple fetch task is a QUERY
+ * empty string if we can't determine the type of the query,
+ * e.g. when ParseException happens, we won't do further magic,
+ * even if it's obvious by reading the sql statement that user wanted to run
e.g. a select query
+ * @param tree the root ASTNode of the query
+ */
+ public void setQueryType(ASTNode tree) {
+ List<Task<? extends Serializable>> rootTasks = getAllRootTasks();
+ if (queryProperties == null) {
+ return;
+ }
+ queryProperties.setQueryType(queryProperties.isAnalyze() ? QueryType.STATS
:
Review Comment:
regarding the massive logic: according to the current SA hierarchy, we
cannot tell query type easily from class, the base class handles too many
cases, and this could be the only place to handle all the dirty exceptions,
like:
```
if ("TOK_QUERY".equalsIgnoreCase(tree.getText())) {
return QueryType.QUERY;
}
// CREATE TABLE is a DDL and yet it's not handled by
DDLSemanticAnalyzerFactory
// FIXME: HIVE-28724
if ("TOK_CREATETABLE".equalsIgnoreCase(tree.getText())) {
return QueryType.DDL;
}
// from this point, best efforts come
// DDL semantic analyzers don't have a single common place to override
this method, hence handling here
if (DDLSemanticAnalyzerFactory.handles(tree)) {
return QueryType.DDL;
}
// CREATE MV is handled by the base semantic analyzer, need to handle
here
if ("TOK_CREATE_MATERIALIZED_VIEW".equalsIgnoreCase(tree.getText())) {
return QueryType.DDL;
}
// if there is a fetch task, this is a query
if (getFetchTask() != null) {
return QueryType.QUERY;
}
```
yeah, it's better as protected, let me rewrite
regarding DLL: we do horrible things to QueryProperties during the analysis,
I mean, clearing it in CalcitePlanner at a given point of time, so we cannot do
something like this while instantiating, because we cannot make sure it's there
at the end of the analysis, that's why post-analysis handling of this whole
thing can work
```
DDLSemanticAnalyzerFactory.getAnalyzer(
...
analyzer.setQueryType(QueryType.DDL);
}
```
AcidExportSemanticAnalyzer is OTHER, ack, need to implement that method
--
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: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
