diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index d1772268c2..123a1f7529 100755 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -34,6 +34,7 @@ import org.apache.impala.analysis.ColumnDef.Option; import org.apache.impala.analysis.SetOperationStmt.Qualifier; import org.apache.impala.analysis.SetOperationStmt.SetOperand; import org.apache.impala.analysis.SetOperationStmt.SetOperator; +import org.apache.impala.analysis.RangeBound; import org.apache.impala.analysis.RangePartition; import org.apache.impala.analysis.TableSampleClause; import org.apache.impala.analysis.AlterTableAddDropRangePartitionStmt.Operation; @@ -545,8 +546,7 @@ nonterminal StructField struct_field_def; nonterminal KuduPartitionParam hash_partition_param; nonterminal List range_params_list; nonterminal RangePartition range_param; -nonterminal Pair, Boolean> opt_lower_range_val, - opt_upper_range_val; +nonterminal RangeBound opt_lower_range_val, opt_upper_range_val; nonterminal List hash_partition_param_list, custom_hash_partition_param_list; nonterminal List partition_param_list; nonterminal KuduPartitionParam range_partition_param; @@ -2050,7 +2050,8 @@ range_params_list ::= range_param ::= KW_PARTITION opt_lower_range_val:lower_val KW_VALUES opt_upper_range_val:upper_val custom_hash_partition_param_list:hashspec - {: RESULT = RangePartition.createFromRange(lower_val, upper_val, hashspec); :} + {: RESULT = RangePartition.createFromRangeWithNormalization( + lower_val, upper_val, hashspec); :} // Use dotted_path to avoid reduce/reduce conflicts with expr | KW_PARTITION dotted_path:val EQUAL expr:l custom_hash_partition_param_list:hashspec {: @@ -2081,26 +2082,48 @@ custom_hash_partition_param_list ::= opt_lower_range_val ::= expr:l LESSTHAN - {: RESULT = new Pair, Boolean>(Lists.newArrayList(l), false); :} + {: RESULT = new RangeBound(Lists.newArrayList(l), false); :} | expr:l LESSTHAN EQUAL - {: RESULT = new Pair, Boolean>(Lists.newArrayList(l), true); :} + {: RESULT = new RangeBound(Lists.newArrayList(l), true); :} | LPAREN expr_list:l RPAREN LESSTHAN - {: RESULT = new Pair, Boolean>(l, false); :} + {: RESULT = new RangeBound(l, false); :} | LPAREN expr_list:l RPAREN LESSTHAN EQUAL - {: RESULT = new Pair, Boolean>(l, true); :} + {: RESULT = new RangeBound(l, true); :} + // Reversed comparator forms (IMPALA-7618): expr >= VALUES, expr > VALUES + // These are syntactically in the lower bound position but semantically define + // an upper bound. The 'reversed' flag signals this to the range_param rule. + | expr:l GREATERTHAN EQUAL + {: RESULT = new RangeBound(Lists.newArrayList(l), true, true); :} + | expr:l GREATERTHAN + {: RESULT = new RangeBound(Lists.newArrayList(l), false, true); :} + | LPAREN expr_list:l RPAREN GREATERTHAN EQUAL + {: RESULT = new RangeBound(l, true, true); :} + | LPAREN expr_list:l RPAREN GREATERTHAN + {: RESULT = new RangeBound(l, false, true); :} | /* empty */ {: RESULT = null; :} ; opt_upper_range_val ::= LESSTHAN expr:l - {: RESULT = new Pair, Boolean>(Lists.newArrayList(l), false); :} + {: RESULT = new RangeBound(Lists.newArrayList(l), false); :} | LESSTHAN EQUAL expr:l - {: RESULT = new Pair, Boolean>(Lists.newArrayList(l), true); :} + {: RESULT = new RangeBound(Lists.newArrayList(l), true); :} | LESSTHAN LPAREN expr_list:l RPAREN - {: RESULT = new Pair, Boolean>(l, false); :} + {: RESULT = new RangeBound(l, false); :} | LESSTHAN EQUAL LPAREN expr_list:l RPAREN - {: RESULT = new Pair, Boolean>(l, true); :} + {: RESULT = new RangeBound(l, true); :} + // Reversed comparator forms (IMPALA-7618): VALUES >= expr, VALUES > expr + // These are syntactically in the upper bound position but semantically define + // a lower bound. The 'reversed' flag signals this to the range_param rule. + | GREATERTHAN EQUAL expr:l + {: RESULT = new RangeBound(Lists.newArrayList(l), true, true); :} + | GREATERTHAN expr:l + {: RESULT = new RangeBound(Lists.newArrayList(l), false, true); :} + | GREATERTHAN EQUAL LPAREN expr_list:l RPAREN + {: RESULT = new RangeBound(l, true, true); :} + | GREATERTHAN LPAREN expr_list:l RPAREN + {: RESULT = new RangeBound(l, false, true); :} | /* empty */ {: RESULT = null; :} ; diff --git a/fe/src/main/java/org/apache/impala/analysis/RangeBound.java b/fe/src/main/java/org/apache/impala/analysis/RangeBound.java new file mode 100644 index 0000000000..54a67a8782 --- /dev/null +++ b/fe/src/main/java/org/apache/impala/analysis/RangeBound.java @@ -0,0 +1,49 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.impala.analysis; + +import java.util.List; + +/** + * Represents a parsed range partition bound value with its inclusiveness and + * whether it was specified in reversed operator form (e.g., VALUES >= X instead + * of X <= VALUES). The 'reversed' flag indicates that the bound was specified in + * a position opposite to its semantic meaning and needs to be swapped during + * range partition construction. + */ +public class RangeBound { + private final List values_; + private final boolean inclusive_; + // True if this bound was specified with a reversed comparator (> or >=) + // and needs to be moved to the opposite bound position. + private final boolean reversed_; + + public RangeBound(List values, boolean inclusive, boolean reversed) { + values_ = values; + inclusive_ = inclusive; + reversed_ = reversed; + } + + public RangeBound(List values, boolean inclusive) { + this(values, inclusive, false); + } + + public List getValues() { return values_; } + public boolean isInclusive() { return inclusive_; } + public boolean isReversed() { return reversed_; } +} diff --git a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java index a0273e1065..3963e0de40 100644 --- a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java +++ b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java @@ -27,7 +27,7 @@ import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.InternalException; -import org.apache.impala.common.Pair; + import org.apache.impala.thrift.TRangePartition; import org.apache.impala.util.ExprUtil; import org.apache.impala.util.KuduUtil; @@ -52,6 +52,11 @@ * PARTITION VALUE = val * PARTITION VALUE = (val1, val2, ..., valn) * + * Additionally, reversed comparator forms are accepted (IMPALA-7618) and normalized + * to the canonical form above: + * PARTITION VALUES >[=] l_val (equivalent to l_val <[=] VALUES) + * PARTITION u_val >[=] VALUES (equivalent to VALUES <[=] u_val) + * * Internally, all these cases are represented using the quadruplet: * [(l_val1,..., l_valn), l_bound_type, (u_val1,..., u_valn), u_bound_type], * where l_bound_type, u_bound_type are boolean values indicating if the associated bounds @@ -87,27 +92,38 @@ private RangePartition(List lowerBoundValues, boolean lowerBoundInclusive, } /** - * Constructs a range partition. The range is specified in the CREATE/ALTER TABLE - * statement using the 'PARTITION OP VALUES OP ' clause or using the - * 'PARTITION (,....,) OP VALUES OP (,...,)' clause. 'lower' - * corresponds to the ' OP' or '(,...,) OP' pair which defines an - * optional lower bound, and similarly 'upper' corresponds to the optional upper bound. - * Since only '<' and '<=' operators are allowed, operators are represented with boolean - * values that indicate inclusive or exclusive bounds. + * Constructs a range partition from RangeBound objects parsed by the SQL grammar. + * Handles normalization of reversed comparator forms (IMPALA-7618): when a bound + * was specified with '>' or '>=' operators (e.g., 'VALUES >= X' or 'X > VALUES'), + * the RangeBound's reversed flag is set, and this method swaps it to the correct + * position. For example, 'VALUES >= X' appears in the upper bound position but + * semantically defines a lower bound (inclusive), so it is moved to the lower + * bound position. */ - public static RangePartition createFromRange(Pair, Boolean> lower, - Pair, Boolean> upper, List hashSpec) { + public static RangePartition createFromRangeWithNormalization(RangeBound lower, + RangeBound upper, List hashSpec) { + // Normalize reversed bounds: a reversed lower bound is actually an upper bound + // and vice versa. Swap them into the correct positions. + RangeBound actualLower = null; + RangeBound actualUpper = null; + if (lower != null && !lower.isReversed()) actualLower = lower; + if (upper != null && !upper.isReversed()) actualUpper = upper; + // A reversed bound in the lower position is actually an upper bound. + if (lower != null && lower.isReversed()) actualUpper = lower; + // A reversed bound in the upper position is actually a lower bound. + if (upper != null && upper.isReversed()) actualLower = upper; + List lowerBoundExprs = Lists.newArrayListWithCapacity(1); boolean lowerBoundInclusive = false; List upperBoundExprs = Lists.newArrayListWithCapacity(1); boolean upperBoundInclusive = false; - if (lower != null) { - lowerBoundExprs = lower.first; - lowerBoundInclusive = lower.second; + if (actualLower != null) { + lowerBoundExprs = actualLower.getValues(); + lowerBoundInclusive = actualLower.isInclusive(); } - if (upper != null) { - upperBoundExprs = upper.first; - upperBoundInclusive = upper.second; + if (actualUpper != null) { + upperBoundExprs = actualUpper.getValues(); + upperBoundInclusive = actualUpper.isInclusive(); } return new RangePartition(lowerBoundExprs, lowerBoundInclusive, upperBoundExprs, upperBoundInclusive, hashSpec); diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java index 1a1d02b073..84dbf55a9e 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java @@ -188,6 +188,18 @@ private void testDDlsOnKuduTable(boolean isExternalPurgeTbl) { AnalyzesOk("create table tab (id int, name string, valf float, vali bigint, " + "primary key (id, name)) partition by range (name) " + "(partition 'aa' < values <= 'bb') stored as kudu", isExternalPurgeTbl); + // Reversed comparator forms (IMPALA-7618): '>' and '>=' should be accepted + // and produce the same internal representation as the canonical '<' / '<=' forms. + AnalyzesOk("create table tab (a int, b int, c int, d int, primary key(a, b, c, d))" + + "partition by hash (a, b, c) partitions 8, " + + "range (a) (partition values >= 1, partition 4 > values) " + + "stored as kudu", isExternalPurgeTbl); + AnalyzesOk("create table tab (id int, name string, primary key(id, name)) " + + "partition by range (name) " + + "(partition values >= 'aa') stored as kudu", isExternalPurgeTbl); + AnalyzesOk("create table tab (id int, name string, primary key(id, name)) " + + "partition by range (name) " + + "(partition 'zz' >= values) stored as kudu", isExternalPurgeTbl); // Null values in range partition values AnalysisError("create table tab (id int, name string, primary key(id, name)) " + "partition by hash (id) partitions 3, range (name) " + diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java index b81a7bac8a..a71b54e678 100644 --- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java @@ -2529,6 +2529,15 @@ public void TestAlterTableAddPartition() { option)); ParsesOk(String.format("ALTER TABLE Foo ADD %s RANGE PARTITION VALUE = 100", option)); + // Reversed comparator forms (IMPALA-7618) + ParsesOk(String.format( + "ALTER TABLE Foo ADD %s RANGE PARTITION VALUES >= 10", option)); + ParsesOk(String.format( + "ALTER TABLE Foo ADD %s RANGE PARTITION VALUES > 10", option)); + ParsesOk(String.format( + "ALTER TABLE Foo ADD %s RANGE PARTITION 20 >= VALUES", option)); + ParsesOk(String.format( + "ALTER TABLE Foo ADD %s RANGE PARTITION 20 > VALUES", option)); ParserError(String.format("ALTER TABLE Foo ADD %s RANGE PARTITION 10 < VALUES " + "<= 20, PARTITION 20 < VALUES <= 30", option)); ParserError(String.format("ALTER TABLE Foo ADD %s (RANGE PARTITION 10 < VALUES " + @@ -2619,6 +2628,15 @@ public void TestAlterTableDropPartition() { option)); ParsesOk(String.format("ALTER TABLE Foo DROP %s RANGE PARTITION VALUE = 100", option)); + // Reversed comparator forms (IMPALA-7618) + ParsesOk(String.format( + "ALTER TABLE Foo DROP %s RANGE PARTITION VALUES >= 10", option)); + ParsesOk(String.format( + "ALTER TABLE Foo DROP %s RANGE PARTITION VALUES > 10", option)); + ParsesOk(String.format( + "ALTER TABLE Foo DROP %s RANGE PARTITION 20 >= VALUES", option)); + ParsesOk(String.format( + "ALTER TABLE Foo DROP %s RANGE PARTITION 20 > VALUES", option)); ParserError(String.format("ALTER TABLE Foo DROP %s RANGE PARTITION 10 < VALUES " + "<= 20, PARTITION 20 < VALUES <= 30", option)); ParserError(String.format("ALTER TABLE Foo DROP %s (RANGE PARTITION 10 < VALUES " + @@ -3188,6 +3206,19 @@ public void TestCreateTable() { ParsesOk("CREATE TABLE Foo (a int) PARTITION BY RANGE (a) " + "(PARTITION now() <= VALUES, PARTITION VALUE = add_months(now(), 2)) " + "STORED AS KUDU"); + // Reversed comparator forms (IMPALA-7618) + ParsesOk("CREATE TABLE Foo (a int) PARTITION BY RANGE(a) " + + "(PARTITION VALUES >= 10)"); + ParsesOk("CREATE TABLE Foo (a int) PARTITION BY RANGE(a) " + + "(PARTITION VALUES > 10)"); + ParsesOk("CREATE TABLE Foo (a int) PARTITION BY RANGE(a) " + + "(PARTITION 20 >= VALUES)"); + ParsesOk("CREATE TABLE Foo (a int) PARTITION BY RANGE(a) " + + "(PARTITION 20 > VALUES)"); + ParsesOk("CREATE TABLE Foo (a int, b int) PARTITION BY RANGE(a, b) " + + "(PARTITION VALUES >= (1, 2))"); + ParsesOk("CREATE TABLE Foo (a int, b int) PARTITION BY RANGE(a, b) " + + "(PARTITION (1, 2) >= VALUES)"); ParserError("CREATE TABLE Foo (a int) PARTITION BY RANGE (a) ()"); ParserError("CREATE TABLE Foo (a int) PARTITION BY HASH (a) PARTITIONS 4, " +