This is an automated email from the ASF dual-hosted git repository.
kgyrtkirk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 587b402 HIVE-24031: Infinite planning time on syntactically big
queries (#1424) (Stamatis Zampetakis reviewed by Zoltan Haindrich)
587b402 is described below
commit 587b402aa6f5357cf6bd16893606b48b3406e9e5
Author: Stamatis Zampetakis <zabe...@gmail.com>
AuthorDate: Tue Sep 8 18:45:23 2020 +0200
HIVE-24031: Infinite planning time on syntactically big queries (#1424)
(Stamatis Zampetakis reviewed by Zoltan Haindrich)
1. Drop the defensive copy of children inside ASTNode#getChildren.
2. Protect clients by accidentally modifying the list via an
unmodifiable collection.
Profiling shows the vast majority of time spend on creating defensive
copies of the node expression list inside ASTNode#getChildren.
The method is called extensively from various places in the code
especially those walking over the expression tree so it needs to be
efficient.
Most of the time creating defensive copies is not necessary. For those
cases (if any) that the list needs to be modified clients should perform
a copy themselves.
---
parser/src/java/org/apache/hadoop/hive/ql/parse/ASTNode.java | 9 +++++++--
.../apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java | 2 +-
.../storage/skewed/AlterTableSetSkewedLocationAnalyzer.java | 2 +-
.../org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java | 2 +-
.../java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java | 2 +-
5 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/parser/src/java/org/apache/hadoop/hive/ql/parse/ASTNode.java
b/parser/src/java/org/apache/hadoop/hive/ql/parse/ASTNode.java
index 2d611fb..f51de08 100644
--- a/parser/src/java/org/apache/hadoop/hive/ql/parse/ASTNode.java
+++ b/parser/src/java/org/apache/hadoop/hive/ql/parse/ASTNode.java
@@ -21,6 +21,7 @@ package org.apache.hadoop.hive.ql.parse;
import java.io.Serializable;
import java.util.ArrayDeque;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Deque;
import java.util.List;
@@ -74,11 +75,15 @@ public class ASTNode extends CommonTree implements
Node,Serializable {
* @see org.apache.hadoop.hive.ql.lib.Node#getChildren()
*/
@Override
- public ArrayList<Node> getChildren() {
+ public List<Node> getChildren() {
if (super.getChildCount() == 0) {
return null;
}
- return new ArrayList<>((List<? extends Node>) super.getChildren());
+ // We know that children always contains Node instances so the cast is
safe.
+ assert this.children.get(0) instanceof Node;
+ @SuppressWarnings("unchecked") // Cast safe by design of the class
+ List<Node> nodeList = (List<Node>) (List<?>) this.children;
+ return Collections.unmodifiableList(nodeList);
}
/*
diff --git
a/parser/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java
b/parser/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java
index 3abaf24..fd3cc76 100644
---
a/parser/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java
+++
b/parser/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java
@@ -70,7 +70,7 @@ public class TestParseDriverIntervals {
return n;
}
}
- ArrayList<Node> children = n.getChildren();
+ List<Node> children = n.getChildren();
if (children != null) {
for (Node c : children) {
ASTNode r = findFunctionNode((ASTNode) c);
diff --git
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/skewed/AlterTableSetSkewedLocationAnalyzer.java
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/skewed/AlterTableSetSkewedLocationAnalyzer.java
index 488f76e..d9fe352 100644
---
a/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/skewed/AlterTableSetSkewedLocationAnalyzer.java
+++
b/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/skewed/AlterTableSetSkewedLocationAnalyzer.java
@@ -56,7 +56,7 @@ public class AlterTableSetSkewedLocationAnalyzer extends
AbstractAlterTableAnaly
@Override
protected void analyzeCommand(TableName tableName, Map<String, String>
partitionSpec, ASTNode command)
throws SemanticException {
- ArrayList<Node> locationNodes = command.getChildren();
+ List<Node> locationNodes = command.getChildren();
if (locationNodes == null) {
throw new
SemanticException(ErrorMsg.ALTER_TBL_SKEWED_LOC_NO_LOC.getMsg());
}
diff --git
a/ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java
b/ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java
index cea7b11..bdcbf62 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/MergeSemanticAnalyzer.java
@@ -593,7 +593,7 @@ public class MergeSemanticAnalyzer extends
RewriteSemanticAnalyzer {
assert whenClauseOperation.getType() == HiveParser.TOK_INSERT;
// identify the node that contains the values to insert and the optional
column list node
- ArrayList<Node> children = whenClauseOperation.getChildren();
+ List<Node> children = whenClauseOperation.getChildren();
ASTNode valuesNode =
(ASTNode)children.stream().filter(n -> ((ASTNode)n).getType() ==
HiveParser.TOK_FUNCTION).findFirst().get();
ASTNode columnListNode =
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
index 8308d9f..ad3092e 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
@@ -14000,7 +14000,7 @@ public class SemanticAnalyzer extends
BaseSemanticAnalyzer {
}
}
- ArrayList childrenList = next.getChildren();
+ List<Node> childrenList = next.getChildren();
for (int i = childrenList.size() - 1; i >= 0; i--) {
stack.push((ASTNode)childrenList.get(i));
}
