IMPALA-7618: Accept reversed comparators (> / >=) in range partition bounds

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<RangePartition> range_params_list;
 nonterminal RangePartition range_param;
-nonterminal Pair<List<Expr>, Boolean> opt_lower_range_val,
-   opt_upper_range_val;
+nonterminal RangeBound opt_lower_range_val, opt_upper_range_val;
 nonterminal List<KuduPartitionParam> hash_partition_param_list, custom_hash_partition_param_list;
 nonterminal List<KuduPartitionParam> 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<List<Expr>, Boolean>(Lists.newArrayList(l), false); :}
+  {: RESULT = new RangeBound(Lists.newArrayList(l), false); :}
   | expr:l LESSTHAN EQUAL
-  {: RESULT = new Pair<List<Expr>, Boolean>(Lists.newArrayList(l), true); :}
+  {: RESULT = new RangeBound(Lists.newArrayList(l), true); :}
   | LPAREN expr_list:l RPAREN LESSTHAN
-  {: RESULT = new Pair<List<Expr>, Boolean>(l, false); :}
+  {: RESULT = new RangeBound(l, false); :}
   | LPAREN expr_list:l RPAREN LESSTHAN EQUAL
-  {: RESULT = new Pair<List<Expr>, 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<List<Expr>, Boolean>(Lists.newArrayList(l), false); :}
+  {: RESULT = new RangeBound(Lists.newArrayList(l), false); :}
   | LESSTHAN EQUAL expr:l
-  {: RESULT = new Pair<List<Expr>, Boolean>(Lists.newArrayList(l), true); :}
+  {: RESULT = new RangeBound(Lists.newArrayList(l), true); :}
   | LESSTHAN LPAREN expr_list:l RPAREN
-  {: RESULT = new Pair<List<Expr>, Boolean>(l, false); :}
+  {: RESULT = new RangeBound(l, false); :}
   | LESSTHAN EQUAL LPAREN expr_list:l RPAREN
-  {: RESULT = new Pair<List<Expr>, 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; :}
   ;

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<Expr> 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<Expr> values, boolean inclusive, boolean reversed) {
+    values_ = values;
+    inclusive_ = inclusive;
+    reversed_ = reversed;
+  }
+
+  public RangeBound(List<Expr> values, boolean inclusive) {
+    this(values, inclusive, false);
+  }
+
+  public List<Expr> getValues() { return values_; }
+  public boolean isInclusive() { return inclusive_; }
+  public boolean isReversed() { return reversed_; }
+}

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<Expr> lowerBoundValues, boolean lowerBoundInclusive,
   }
 
   /**
-   * Constructs a range partition. The range is specified in the CREATE/ALTER TABLE
-   * statement using the 'PARTITION <expr> OP VALUES OP <expr>' clause or using the
-   * 'PARTITION (<expr>,....,<expr>) OP VALUES OP (<expr>,...,<expr>)' clause. 'lower'
-   * corresponds to the '<expr> OP' or '(<expr>,...,<expr>) 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<List<Expr>, Boolean> lower,
-      Pair<List<Expr>, Boolean> upper, List<KuduPartitionParam> hashSpec) {
+  public static RangePartition createFromRangeWithNormalization(RangeBound lower,
+      RangeBound upper, List<KuduPartitionParam> 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<Expr> lowerBoundExprs = Lists.newArrayListWithCapacity(1);
     boolean lowerBoundInclusive = false;
     List<Expr> 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);

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) " +

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, " +