HIVE-10963 : Hive throws NPE rather than meaningful error message when window is missing (Aihua Xu via Ashutosh Chauhan)
Signed-off-by: Ashutosh Chauhan <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/f2600e9d Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/f2600e9d Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/f2600e9d Branch: refs/heads/llap Commit: f2600e9da833f409afc33be9f92db4dbe9ad2c9b Parents: 19d7074 Author: Aihua Xu <[email protected]> Authored: Tue Jun 9 07:41:00 2015 -0700 Committer: Ashutosh Chauhan <[email protected]> Committed: Wed Jun 10 00:13:46 2015 -0700 ---------------------------------------------------------------------- .../hadoop/hive/ql/parse/CalcitePlanner.java | 6 +-- .../hadoop/hive/ql/parse/PTFInvocationSpec.java | 8 ++++ .../hadoop/hive/ql/parse/WindowingSpec.java | 46 ++++++++++---------- .../clientnegative/ptf_negative_NoWindowDefn.q | 9 ++++ .../ptf_negative_NoWindowDefn.q.out | 1 + 5 files changed, 43 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/f2600e9d/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java index 48e3cc7..bff9772 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java @@ -2378,9 +2378,9 @@ public class CalcitePlanner extends SemanticAnalyzer { WindowSpec wndSpec = ((WindowFunctionSpec) wExpSpec).getWindowSpec(); List<RexNode> partitionKeys = getPartitionKeys(wndSpec.getPartition(), converter, inputRR); List<RexFieldCollation> orderKeys = getOrderKeys(wndSpec.getOrder(), converter, inputRR); - RexWindowBound upperBound = getBound(wndSpec.windowFrame.start, converter); - RexWindowBound lowerBound = getBound(wndSpec.windowFrame.end, converter); - boolean isRows = ((wndSpec.windowFrame.start instanceof RangeBoundarySpec) || (wndSpec.windowFrame.end instanceof RangeBoundarySpec)) ? true + RexWindowBound upperBound = getBound(wndSpec.getWindowFrame().start, converter); + RexWindowBound lowerBound = getBound(wndSpec.getWindowFrame().end, converter); + boolean isRows = ((wndSpec.getWindowFrame().start instanceof RangeBoundarySpec) || (wndSpec.getWindowFrame().end instanceof RangeBoundarySpec)) ? true : false; w = cluster.getRexBuilder().makeOver(calciteAggFnRetType, calciteAggFn, calciteAggFnArgs, http://git-wip-us.apache.org/repos/asf/hive/blob/f2600e9d/ql/src/java/org/apache/hadoop/hive/ql/parse/PTFInvocationSpec.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/PTFInvocationSpec.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/PTFInvocationSpec.java index 06d3f4b..29b8510 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/PTFInvocationSpec.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/PTFInvocationSpec.java @@ -202,6 +202,7 @@ public class PTFInvocationSpec { public static class PartitioningSpec { PartitionSpec partSpec; OrderSpec orderSpec; + public PartitionSpec getPartSpec() { return partSpec; } @@ -250,6 +251,13 @@ public class PTFInvocationSpec { } return true; } + + @Override + public String toString() { + return String.format("PartitioningSpec=[%s%s]", + partSpec == null ? "" : partSpec, + orderSpec == null ? "" : orderSpec); + } } /* http://git-wip-us.apache.org/repos/asf/hive/blob/f2600e9d/ql/src/java/org/apache/hadoop/hive/ql/parse/WindowingSpec.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/WindowingSpec.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/WindowingSpec.java index 953f3ae..a181f7c 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/parse/WindowingSpec.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/WindowingSpec.java @@ -58,20 +58,21 @@ import org.apache.hadoop.hive.ql.parse.PTFInvocationSpec.PartitioningSpec; * building RowResolvers. */ public class WindowingSpec { - HashMap<String, WindowExpressionSpec> aliasToWdwExpr; - HashMap<String, WindowSpec> windowSpecs; - ArrayList<WindowExpressionSpec> windowExpressions; + private HashMap<String, WindowExpressionSpec> aliasToWdwExpr; + private HashMap<String, WindowSpec> windowSpecs; + private ArrayList<WindowExpressionSpec> windowExpressions; + + public WindowingSpec() { + aliasToWdwExpr = new HashMap<String, WindowExpressionSpec>(); + windowSpecs = new HashMap<String, WindowSpec>(); + windowExpressions = new ArrayList<WindowExpressionSpec>(); + } public void addWindowSpec(String name, WindowSpec wdwSpec) { - windowSpecs = windowSpecs == null ? new HashMap<String, WindowSpec>() : windowSpecs; windowSpecs.put(name, wdwSpec); } public void addWindowFunction(WindowFunctionSpec wFn) { - windowExpressions = windowExpressions == null ? - new ArrayList<WindowExpressionSpec>() : windowExpressions; - aliasToWdwExpr = aliasToWdwExpr == null ? - new HashMap<String, WindowExpressionSpec>() : aliasToWdwExpr; windowExpressions.add(wFn); aliasToWdwExpr.put(wFn.getAlias(), wFn); } @@ -80,26 +81,14 @@ public class WindowingSpec { return aliasToWdwExpr; } - public void setAliasToWdwExpr(HashMap<String, WindowExpressionSpec> aliasToWdwExpr) { - this.aliasToWdwExpr = aliasToWdwExpr; - } - public HashMap<String, WindowSpec> getWindowSpecs() { return windowSpecs; } - public void setWindowSpecs(HashMap<String, WindowSpec> windowSpecs) { - this.windowSpecs = windowSpecs; - } - public ArrayList<WindowExpressionSpec> getWindowExpressions() { return windowExpressions; } - public void setWindowExpressions(ArrayList<WindowExpressionSpec> windowExpressions) { - this.windowExpressions = windowExpressions; - } - public PartitioningSpec getQueryPartitioningSpec() { /* * Why no null and class checks? @@ -171,7 +160,7 @@ public class WindowingSpec { WindowSpec source = getWindowSpecs().get(sourceId); if (source == null || source.equals(dest)) { - throw new SemanticException(String.format("Window Spec %s refers to an unknown source " , + throw new SemanticException(String.format("%s refers to an unknown source" , dest)); } @@ -445,9 +434,10 @@ public class WindowingSpec { */ public static class WindowSpec { - String sourceId; - PartitioningSpec partitioning; - WindowFrameSpec windowFrame; + private String sourceId; + private PartitioningSpec partitioning; + private WindowFrameSpec windowFrame; + public String getSourceId() { return sourceId; } @@ -496,6 +486,14 @@ public class WindowingSpec { setOrder(order); } } + + @Override + public String toString() { + return String.format("Window Spec=[%s%s%s]", + sourceId == null ? "" : "Name='" + sourceId + "'", + partitioning == null ? "" : partitioning, + windowFrame == null ? "" : windowFrame); + } }; /* http://git-wip-us.apache.org/repos/asf/hive/blob/f2600e9d/ql/src/test/queries/clientnegative/ptf_negative_NoWindowDefn.q ---------------------------------------------------------------------- diff --git a/ql/src/test/queries/clientnegative/ptf_negative_NoWindowDefn.q b/ql/src/test/queries/clientnegative/ptf_negative_NoWindowDefn.q new file mode 100644 index 0000000..8defb3a --- /dev/null +++ b/ql/src/test/queries/clientnegative/ptf_negative_NoWindowDefn.q @@ -0,0 +1,9 @@ +-- testNoWindowDefn +select p_mfgr, p_name, p_size, +sum(p_size) over (w1) as s1, +sum(p_size) over (w2) as s2 +from part +distribute by p_mfgr +sort by p_mfgr +window w1 as (rows between 2 preceding and 2 following); + http://git-wip-us.apache.org/repos/asf/hive/blob/f2600e9d/ql/src/test/results/clientnegative/ptf_negative_NoWindowDefn.q.out ---------------------------------------------------------------------- diff --git a/ql/src/test/results/clientnegative/ptf_negative_NoWindowDefn.q.out b/ql/src/test/results/clientnegative/ptf_negative_NoWindowDefn.q.out new file mode 100644 index 0000000..74b97af --- /dev/null +++ b/ql/src/test/results/clientnegative/ptf_negative_NoWindowDefn.q.out @@ -0,0 +1 @@ +FAILED: SemanticException Window Spec=[Name='w2'] refers to an unknown source
