phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject phoenix git commit: PHOENIX-2120 Padding character is not inverted as required for DESC CHAR columns
Date Wed, 22 Jul 2015 17:20:09 GMT
Repository: phoenix
Updated Branches:
  refs/heads/master 8c2b5a698 -> 498cc5524


PHOENIX-2120 Padding character is not inverted as required for DESC CHAR columns


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/498cc552
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/498cc552
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/498cc552

Branch: refs/heads/master
Commit: 498cc5524ff73c3947160ddfc4241f759c7044f2
Parents: 8c2b5a6
Author: James Taylor <jtaylor@salesforce.com>
Authored: Wed Jul 22 09:18:00 2015 -0700
Committer: James Taylor <jtaylor@salesforce.com>
Committed: Wed Jul 22 09:32:25 2015 -0700

----------------------------------------------------------------------
 .../org/apache/phoenix/end2end/BaseQueryIT.java |  10 --
 .../org/apache/phoenix/end2end/InListIT.java    |  53 +++++++++-
 .../apache/phoenix/end2end/RTrimFunctionIT.java |  71 +++++++++++++
 .../org/apache/phoenix/end2end/SortOrderIT.java |  59 +++++++++++
 .../phoenix/compile/ExpressionCompiler.java     |  12 ++-
 .../apache/phoenix/compile/WhereOptimizer.java  | 102 ++-----------------
 .../apache/phoenix/execute/HashJoinPlan.java    |   6 +-
 .../expression/ArrayConstructorExpression.java  |  28 +++--
 .../phoenix/expression/BaseExpression.java      |  43 ++++----
 .../phoenix/expression/CoerceExpression.java    |  39 ++++---
 .../expression/ComparisonExpression.java        |  32 +++---
 .../phoenix/expression/InListExpression.java    |  26 ++++-
 .../phoenix/expression/LiteralExpression.java   |  18 +++-
 .../expression/function/InvertFunction.java     |   6 +-
 .../expression/function/PrefixFunction.java     |  30 ++++--
 .../expression/function/RTrimFunction.java      |  55 ++++++++--
 .../visitor/CloneExpressionVisitor.java         |   2 +-
 .../phoenix/schema/types/PArrayDataType.java    |   4 +-
 .../java/org/apache/phoenix/query/BaseTest.java |   8 ++
 .../java/org/apache/phoenix/util/TestUtil.java  |   4 +-
 20 files changed, 408 insertions(+), 200 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java
index 5f87e3f..aa5068b 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java
@@ -23,9 +23,7 @@ import java.io.IOException;
 import java.sql.Connection;
 import java.sql.Date;
 import java.sql.DriverManager;
-import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -120,14 +118,6 @@ public abstract class BaseQueryIT extends BaseClientManagedTimeIT {
         return testCases;
     }
     
-    protected void assertValueEqualsResultSet(ResultSet rs, List<Object> expectedResults) throws SQLException {
-        List<List<Object>> nestedExpectedResults = Lists.newArrayListWithExpectedSize(expectedResults.size());
-        for (Object expectedResult : expectedResults) {
-            nestedExpectedResults.add(Arrays.asList(expectedResult));
-        }
-        assertValuesEqualsResultSet(rs, nestedExpectedResults); 
-    }
-
     protected static boolean compare(CompareOp op, ImmutableBytesWritable lhsOutPtr, ImmutableBytesWritable rhsOutPtr) {
         int compareResult = Bytes.compareTo(lhsOutPtr.get(), lhsOutPtr.getOffset(), lhsOutPtr.getLength(), rhsOutPtr.get(), rhsOutPtr.getOffset(), rhsOutPtr.getLength());
         return ByteUtil.compare(op, compareResult);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
index 3fdd906..4aff12b 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
@@ -32,8 +32,9 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.types.PDataType;
+import org.apache.phoenix.schema.types.PInteger;
 import org.apache.phoenix.schema.types.PLong;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.PropertiesUtil;
@@ -428,4 +429,54 @@ public class InListIT extends BaseHBaseManagedTimeIT {
         
         testWithIntegerTypesWithVariedSaltingAndTenancy(DEFAULT_UPSERT_BODIES, whereClause, expecteds);
     }
+    
+    @Test
+    public void testWithFixedLengthDescPK() throws Exception {
+        testWithFixedLengthPK(SortOrder.DESC);
+    }
+    
+    @Test
+    public void testWithFixedLengthAscPK() throws Exception {
+        testWithFixedLengthPK(SortOrder.ASC);        
+    }
+    
+    @Test
+    public void testWithFixedLengthKV() throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        conn.createStatement().execute("CREATE TABLE in_test ( id INTEGER PRIMARY KEY, k CHAR(3))");
+
+        conn.createStatement().execute("upsert into in_test values (1, 'aa')");
+        conn.createStatement().execute("upsert into in_test values (2, 'bb')");
+        conn.commit();
+
+        ResultSet rs = conn.createStatement().executeQuery("select k from in_test WHERE k IN ('aa','bb')");
+        assertTrue(rs.next());
+        assertEquals("aa", rs.getString(1));
+        assertTrue(rs.next());
+        assertEquals("bb", rs.getString(1));
+        assertFalse(rs.next());
+        
+        conn.close();
+    }
+
+    private void testWithFixedLengthPK(SortOrder sortOrder) throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        conn.createStatement().execute("CREATE TABLE in_test ( k CHAR(3) PRIMARY KEY " + (sortOrder == SortOrder.DESC ? "DESC" : "") + ")");
+
+        conn.createStatement().execute("upsert into in_test (k) values ('aa')");
+        conn.createStatement().execute("upsert into in_test (k) values ('bb')");
+        conn.commit();
+
+        ResultSet rs = conn.createStatement().executeQuery("select k from in_test WHERE k IN ('aa','bb')");
+        assertTrue(rs.next());
+        assertEquals(sortOrder == SortOrder.ASC ? "aa" : "bb", rs.getString(1));
+        assertTrue(rs.next());
+        assertEquals(sortOrder == SortOrder.ASC ? "bb" : "aa", rs.getString(1));
+        assertFalse(rs.next());
+        
+        conn.close();
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.java
new file mode 100644
index 0000000..2caca2b
--- /dev/null
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/RTrimFunctionIT.java
@@ -0,0 +1,71 @@
+/*
+ * 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.phoenix.end2end;
+
+import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static org.junit.Assert.assertTrue;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.util.PropertiesUtil;
+import org.apache.phoenix.util.QueryUtil;
+import org.junit.Test;
+
+
+public class RTrimFunctionIT extends BaseHBaseManagedTimeIT {
+    
+    @Test
+    public void testWithFixedLengthAscPK() throws Exception {
+        testWithFixedLengthPK(SortOrder.ASC, Arrays.<Object>asList("b", "b ", "b  "));
+    }
+    
+    @Test
+    public void testWithFixedLengthDescPK() throws Exception {
+        testWithFixedLengthPK(SortOrder.DESC, Arrays.<Object>asList("b  ", "b ", "b"));
+    }
+    
+    private void testWithFixedLengthPK(SortOrder sortOrder, List<Object> expectedResults) throws Exception {
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        Connection conn = DriverManager.getConnection(getUrl(), props);
+        conn.createStatement().execute("CREATE TABLE rtrim_test ( k VARCHAR PRIMARY KEY " + (sortOrder == SortOrder.DESC ? "DESC" : "") + ")");
+
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('a')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('b')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('b ')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('b  ')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('b  a')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values (' b  ')");
+        conn.createStatement().execute("upsert into rtrim_test (k) values ('c')");
+        conn.commit();
+
+        String query = "select k from rtrim_test WHERE rtrim(k)='b'";
+        ResultSet rs = conn.createStatement().executeQuery(query);
+        assertValueEqualsResultSet(rs, expectedResults);
+        
+        rs = conn.createStatement().executeQuery("explain " + query);
+        assertTrue(QueryUtil.getExplainPlan(rs).contains("RANGE SCAN OVER RTRIM_TEST"));
+        
+        conn.close();
+    }
+}

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java
index a03f71e..0e8fb4f 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java
@@ -18,6 +18,8 @@
 package org.apache.phoenix.end2end;
 
 import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -171,6 +173,63 @@ public class SortOrderIT extends BaseHBaseManagedTimeIT {
     }    
     
     @Test
+    public void substrFixedLengthDescPK1() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (oid CHAR(3) PRIMARY KEY DESC)";
+        Object[][] insertedRows = new Object[][]{{"a"}, {"ab"}};
+        Object[][] expectedRows = new Object[][]{{"ab"}, {"a"} };
+        runQueryTest(ddl, upsert("oid"), insertedRows, expectedRows, new WhereCondition("SUBSTR(oid, 1, 1)", "=", "'a'"));
+    }
+        
+    @Test
+    public void substrVarLengthDescPK1() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (oid VARCHAR PRIMARY KEY DESC)";
+        Object[][] insertedRows = new Object[][]{{"a"}, {"ab"}};
+        Object[][] expectedRows = new Object[][]{{"ab"}, {"a"} };
+        runQueryTest(ddl, upsert("oid"), insertedRows, expectedRows, new WhereCondition("SUBSTR(oid, 1, 1)", "=", "'a'"));
+    }
+        
+    @Test
+    public void likeVarLengthDescPK1() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (oid VARCHAR PRIMARY KEY DESC)";
+        Object[][] insertedRows = new Object[][]{{"a"}, {"ab"}};
+        Object[][] expectedRows = new Object[][]{{"ab"}, {"a"} };
+        runQueryTest(ddl, upsert("oid"), insertedRows, expectedRows, new WhereCondition("oid", "like", "'a%'"));
+    }
+        
+    @Test
+    public void likeFixedLengthDescPK1() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (oid CHAR(3) PRIMARY KEY DESC)";
+        Object[][] insertedRows = new Object[][]{{"a"}, {"ab"}};
+        Object[][] expectedRows = new Object[][]{{"ab"}, {"a"} };
+        runQueryTest(ddl, upsert("oid"), insertedRows, expectedRows, new WhereCondition("oid", "like", "'a%'"));
+    }
+        
+    @Test
+    public void decimalRangeDescPK1() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (oid DECIMAL PRIMARY KEY DESC)";
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute(ddl);
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(4.99)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(4.0)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(5.0)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(5.001)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(5.999)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(6.0)");
+        conn.createStatement().execute("UPSERT INTO " + TABLE + " VALUES(6.001)");
+        conn.commit();
+        
+        String query = "SELECT * FROM " + TABLE + " WHERE oid >= 5.0 AND oid < 6.0";
+        ResultSet rs = conn.createStatement().executeQuery(query);
+        assertTrue(rs.next());
+        assertTrue(new BigDecimal("5.999").compareTo(rs.getBigDecimal(1)) == 0);
+        assertTrue(rs.next());
+        assertTrue(new BigDecimal("5.001").compareTo(rs.getBigDecimal(1)) == 0);
+        assertTrue(rs.next());
+        assertTrue(new BigDecimal("5.0").compareTo(rs.getBigDecimal(1)) == 0);
+        assertFalse(rs.next());
+    }
+        
+    @Test
     public void lTrimDescCompositePK() throws Exception {
         String ddl = "CREATE TABLE " + TABLE + " (oid VARCHAR NOT NULL, code INTEGER NOT NULL constraint pk primary key (oid DESC, code DESC))";
         Object[][] insertedRows = new Object[][]{{" o1 ", 1}, {"  o2", 2}, {"  o3", 3}};

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java
index 39baf7a..2f786ec 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/ExpressionCompiler.java
@@ -225,7 +225,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         } else {
             addBindParamMetaData(lhsNode, rhsNode, lhsExpr, rhsExpr);
         }
-        return wrapGroupByExpression(ComparisonExpression.create(op, children, context.getTempPtr()));
+        return wrapGroupByExpression(ComparisonExpression.create(op, children, context.getTempPtr(), context.getCurrentTable().getTable().rowKeyOrderOptimizable()));
     }
 
     @Override
@@ -603,7 +603,8 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
                 expr =  convertToRoundExpressionIfNeeded(fromDataType, targetDataType, children);
             }
         }
-        return wrapGroupByExpression(CoerceExpression.create(expr, targetDataType, SortOrder.getDefault(), expr.getMaxLength()));  
+        boolean rowKeyOrderOptimizable = context.getCurrentTable().getTable().rowKeyOrderOptimizable();
+        return wrapGroupByExpression(CoerceExpression.create(expr, targetDataType, SortOrder.getDefault(), expr.getMaxLength(), rowKeyOrderOptimizable));  
     }
     
    @Override
@@ -632,7 +633,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
                 context.getBindManager().addParamMetaData((BindParseNode)childNode, firstChild);
             }
         }
-        return wrapGroupByExpression(InListExpression.create(inChildren, node.isNegate(), ptr));
+        return wrapGroupByExpression(InListExpression.create(inChildren, node.isNegate(), ptr, context.getCurrentTable().getTable().rowKeyOrderOptimizable()));
     }
 
     private static final PDatum DECIMAL_DATUM = new PDatum() {
@@ -1266,7 +1267,8 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
         // the value object array type should match the java known type
         Object[] elements = (Object[]) java.lang.reflect.Array.newInstance(theArrayElemDataType.getJavaClass(), children.size());
 
-        ArrayConstructorExpression arrayExpression = new ArrayConstructorExpression(children, arrayElemDataType);
+        boolean rowKeyOrderOptimizable = context.getCurrentTable().getTable().rowKeyOrderOptimizable();
+        ArrayConstructorExpression arrayExpression = new ArrayConstructorExpression(children, arrayElemDataType, rowKeyOrderOptimizable);
         if (ExpressionUtil.isConstant(arrayExpression)) {
             for (int i = 0; i < children.size(); i++) {
                 Expression child = children.get(i);
@@ -1281,7 +1283,7 @@ public class ExpressionCompiler extends UnsupportedAllParseNodeVisitor<Expressio
             }
             Object value = PArrayDataType.instantiatePhoenixArray(arrayElemDataType, elements);
             return LiteralExpression.newConstant(value,
-                    PDataType.fromTypeId(arrayElemDataType.getSqlType() + PDataType.ARRAY_TYPE_BASE), Determinism.ALWAYS);
+                    PDataType.fromTypeId(arrayElemDataType.getSqlType() + PDataType.ARRAY_TYPE_BASE), null, null, arrayExpression.getSortOrder(), Determinism.ALWAYS, rowKeyOrderOptimizable);
         }
         
         return wrapGroupByExpression(arrayExpression);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
index a270f12..6dfae7e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
@@ -63,7 +63,6 @@ import org.apache.phoenix.schema.SaltingUtil;
 import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.ValueSchema.Field;
 import org.apache.phoenix.schema.tuple.Tuple;
-import org.apache.phoenix.schema.types.PBinary;
 import org.apache.phoenix.schema.types.PChar;
 import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.schema.types.PVarbinary;
@@ -72,7 +71,6 @@ import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.MetaDataUtil;
 import org.apache.phoenix.util.ScanUtil;
 import org.apache.phoenix.util.SchemaUtil;
-import org.apache.phoenix.util.StringUtil;
 
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
@@ -262,7 +260,6 @@ public class WhereOptimizer {
                     hasNonPointKey = true;
                 }
             }
-            keyRanges = transformKeyRangesIfNecessary(slot, context);
             
             hasMultiRanges |= keyRanges.size() > 1;
             // Force a range scan if we've encountered a multi-span slot (i.e. RVC)
@@ -322,86 +319,6 @@ public class WhereOptimizer {
         }
     }
     
-    private static int computeUnpaddedLength(byte[] b, byte padByte) {
-        int len = b.length;
-        while (len > 0 && b[len - 1] == padByte) {
-            len--;
-        }
-        return len;                                       
-    }
-    
-    // Special hack for PHOENIX-2067 to change the constant array to match
-    // the separators used for descending, variable length arrays.
-    // Note that there'd already be a coerce expression around the constant
-    // to convert it to the right type, so we shouldn't do that here.
-    private static List<KeyRange> transformKeyRangesIfNecessary(KeyExpressionVisitor.KeySlot slot, StatementContext context) {
-        KeyPart keyPart = slot.getKeyPart();
-        List<KeyRange> keyRanges = slot.getKeyRanges();
-        PColumn column = keyPart.getColumn();
-        if (column != null) {
-            PDataType type = column.getDataType();
-            PTable table = context.getCurrentTable().getTable();
-            // Constants are always build with rowKeyOptimizable as true, using the correct separators
-            // We only need to do this conversion if we have a table that has not yet been converted.
-            if (type != null && !table.rowKeyOrderOptimizable()) {
-                if (type.isArrayType() && column.getSortOrder() == SortOrder.DESC) {
-                    ImmutableBytesWritable ptr = context.getTempPtr();
-                    List<KeyRange> newKeyRanges = Lists.newArrayListWithExpectedSize(keyRanges.size());
-                    for (KeyRange keyRange : keyRanges) {
-                        byte[] lower = keyRange.getLowerRange();
-                        if (!keyRange.lowerUnbound()) {
-                            ptr.set(lower);
-                            type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false);
-                            lower = ByteUtil.copyKeyBytesIfNecessary(ptr);
-                        }
-                        byte[] upper = keyRange.getUpperRange();
-                        if (!keyRange.upperUnbound()) {
-                            ptr.set(upper);
-                            type.coerceBytes(ptr, null, type, null, null, SortOrder.DESC, null, null, SortOrder.DESC, false);
-                            upper = ByteUtil.copyKeyBytesIfNecessary(ptr);
-                        }
-                        keyRange = KeyRange.getKeyRange(lower, keyRange.isLowerInclusive(), upper, keyRange.isUpperInclusive());
-                        newKeyRanges.add(keyRange);
-                    }
-                    return newKeyRanges;
-                } else if (type == PBinary.INSTANCE || (column.getSortOrder() == SortOrder.DESC && type == PChar.INSTANCE)) {
-                    // Since the table has not been upgraded, we need to replace the correct trailing byte back to the incorrect
-                    // trailing byte so that we form the correct start/stop key
-                    List<KeyRange> newKeyRanges = Lists.newArrayListWithExpectedSize(keyRanges.size());
-                    byte byteToReplace;
-                    if (type == PBinary.INSTANCE) {
-                        byteToReplace = column.getSortOrder() == SortOrder.ASC ? QueryConstants.SEPARATOR_BYTE : QueryConstants.DESC_SEPARATOR_BYTE;
-                    } else {
-                        byteToReplace = StringUtil.INVERTED_SPACE_UTF8;
-                    }
-                    for (KeyRange keyRange : keyRanges) {
-                        byte[] lower = keyRange.getLowerRange();
-                        if (!keyRange.lowerUnbound()) {
-                            int len = computeUnpaddedLength(lower, byteToReplace);
-                            if (len != lower.length) {
-                                lower = Arrays.copyOf(lower, len);
-                                lower = StringUtil.padChar(lower, lower.length);
-                            }
-                        }
-                        byte[] upper = keyRange.getUpperRange();
-                        if (!keyRange.upperUnbound()) {
-                            int len = computeUnpaddedLength(upper, byteToReplace);
-                            if (len != upper.length) {
-                                upper = Arrays.copyOf(upper, len);
-                                upper = StringUtil.padChar(upper, upper.length);
-                            }
-                        }
-                        keyRange = KeyRange.getKeyRange(lower, keyRange.isLowerInclusive(), upper, keyRange.isUpperInclusive());
-                        newKeyRanges.add(keyRange);
-                    }
-                    return newKeyRanges;
-                }
-                
-            }
-        }
-        return keyRanges;
-    }
-    
     /**
      * Get an optimal combination of key expressions for hash join key range optimization.
      * @return returns true if the entire combined expression is covered by key range optimization
@@ -464,7 +381,7 @@ public class WhereOptimizer {
             Expression lhs = count == 0 ? candidates.get(0) : new RowValueConstructorExpression(candidates.subList(0, count + 1), false);
             Expression firstRhs = count == 0 ? sampleValues.get(0).get(0) : new RowValueConstructorExpression(sampleValues.get(0).subList(0, count + 1), true);
             Expression secondRhs = count == 0 ? sampleValues.get(1).get(0) : new RowValueConstructorExpression(sampleValues.get(1).subList(0, count + 1), true);
-            Expression testExpression = InListExpression.create(Lists.newArrayList(lhs, firstRhs, secondRhs), false, context.getTempPtr());
+            Expression testExpression = InListExpression.create(Lists.newArrayList(lhs, firstRhs, secondRhs), false, context.getTempPtr(), context.getCurrentTable().getTable().rowKeyOrderOptimizable());
             remaining = pushKeyExpressionsToScan(context, statement, testExpression);
             if (context.getScanRanges().isPointLookup()) {
                 count++;
@@ -1060,16 +977,19 @@ public class WhereOptimizer {
             byte[] lowerRange = key;
             byte[] upperRange = ByteUtil.nextKey(key);
             Integer columnFixedLength = column.getMaxLength();
-            if (type.isFixedWidth() && columnFixedLength != null) {
-                if (table.rowKeyOrderOptimizable()) {
+            if (type.isFixedWidth()) {
+                if (columnFixedLength != null) { // Sanity check - should always be non null
                     // Always use minimum byte to fill as otherwise our key is bigger
                     // that it should be when the sort order is descending.
                     lowerRange = type.pad(lowerRange, columnFixedLength, SortOrder.ASC);
                     upperRange = type.pad(upperRange, columnFixedLength, SortOrder.ASC);
-                } else { // TODO: remove broken logic once tables are required to have been upgraded for PHOENIX-2067 and PHOENIX-2120
-                    lowerRange = StringUtil.padChar(lowerRange, columnFixedLength);
-                    upperRange = StringUtil.padChar(upperRange, columnFixedLength);
                 }
+            } else if (column.getSortOrder() == SortOrder.DESC && table.rowKeyOrderOptimizable()) {
+                // Append a zero byte if descending since a \xFF byte will be appended to the lowerRange
+                // causing rows to be skipped that should be included. For example, with rows 'ab', 'a',
+                // a lowerRange of 'a\xFF' would skip 'ab', while 'a\x00\xFF' would not.
+                lowerRange = Arrays.copyOf(lowerRange, lowerRange.length+1);
+                lowerRange[lowerRange.length-1] = QueryConstants.SEPARATOR_BYTE;
             }
             KeyRange keyRange = type.getKeyRange(lowerRange, true, upperRange, false);
             // Only extract LIKE expression if pattern ends with a wildcard and everything else was extracted
@@ -1430,7 +1350,7 @@ public class WhereOptimizer {
                     rhs = BaseExpression.coerce(rvc, rhs, new ExpressionComparabilityWrapper() {
 
                         @Override
-                        public Expression wrap(final Expression lhs, final Expression rhs) throws SQLException {
+                        public Expression wrap(final Expression lhs, final Expression rhs, boolean rowKeyOrderOptimizable) throws SQLException {
                             final KeyPart childPart = keySlotsIterator.next().iterator().next().getKeyPart();
                             // TODO: DelegateExpression
                             return new BaseTerminalExpression() {
@@ -1502,7 +1422,7 @@ public class WhereOptimizer {
                             };
                         }
                         
-                    });
+                    }, table.rowKeyOrderOptimizable());
                 } catch (SQLException e) {
                     return null; // Shouldn't happen
                 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java
index 05ef1ec..2ac728d 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/HashJoinPlan.java
@@ -208,13 +208,13 @@ public class HashJoinPlan extends DelegateQueryPlan {
 
     private Expression createKeyRangeExpression(Expression lhsExpression,
             Expression rhsExpression, List<Expression> rhsValues, 
-            ImmutableBytesWritable ptr) throws SQLException {
+            ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
         if (rhsValues.isEmpty())
             return LiteralExpression.newConstant(false, PBoolean.INSTANCE, Determinism.ALWAYS);        
         
         rhsValues.add(0, lhsExpression);
         
-        return InListExpression.create(rhsValues, false, ptr);
+        return InListExpression.create(rhsValues, false, ptr, rowKeyOrderOptimizable);
     }
 
     @Override
@@ -366,7 +366,7 @@ public class HashJoinPlan extends DelegateQueryPlan {
                 }
             }
             if (keyRangeRhsValues != null) {
-                parent.keyRangeExpressions.add(parent.createKeyRangeExpression(keyRangeLhsExpression, keyRangeRhsExpression, keyRangeRhsValues, plan.getContext().getTempPtr()));
+                parent.keyRangeExpressions.add(parent.createKeyRangeExpression(keyRangeLhsExpression, keyRangeRhsExpression, keyRangeRhsValues, plan.getContext().getTempPtr(), plan.getContext().getCurrentTable().getTable().rowKeyOrderOptimizable()));
             }
             return cache;
         }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java
index d1b4a0e..d8df29a 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/ArrayConstructorExpression.java
@@ -19,7 +19,6 @@ import java.util.List;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.io.WritableUtils;
 import org.apache.phoenix.expression.visitor.ExpressionVisitor;
-import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.tuple.Tuple;
 import org.apache.phoenix.schema.types.PArrayDataType;
 import org.apache.phoenix.schema.types.PDataType;
@@ -38,21 +37,23 @@ public class ArrayConstructorExpression extends BaseCompoundExpression {
     // store the offset postion in this.  Later based on the total size move this to a byte[]
     // and serialize into byte stream
     private int[] offsetPos;
+    private boolean rowKeyOrderOptimizable;
     
     public ArrayConstructorExpression() {
     }
 
-    public ArrayConstructorExpression(List<Expression> children, PDataType baseType) {
+    public ArrayConstructorExpression(List<Expression> children, PDataType baseType, boolean rowKeyOrderOptimizable) {
         super(children);
-        init(baseType);
+        init(baseType, rowKeyOrderOptimizable);
     }
 
     public ArrayConstructorExpression clone(List<Expression> children) {
-        return new ArrayConstructorExpression(children, this.baseType);
+        return new ArrayConstructorExpression(children, this.baseType, this.rowKeyOrderOptimizable);
     }
     
-    private void init(PDataType baseType) {
+    private void init(PDataType baseType, boolean rowKeyOrderOptimizable) {
         this.baseType = baseType;
+        this.rowKeyOrderOptimizable = rowKeyOrderOptimizable;
         elements = new Object[getChildren().size()];
         valuePtr.set(ByteUtil.EMPTY_BYTE_ARRAY);
         estimatedSize = PArrayDataType.estimateSize(this.children.size(), this.baseType);
@@ -113,7 +114,7 @@ public class ArrayConstructorExpression extends BaseCompoundExpression {
                             PArrayDataType.serializeNulls(oStream, nNulls);
                             offsetPos[i] = byteStream.size();
                             oStream.write(ptr.get(), ptr.getOffset(), ptr.getLength());
-                            oStream.write(QueryConstants.SEPARATOR_BYTE);
+                            oStream.write(PArrayDataType.getSeparatorByte(rowKeyOrderOptimizable, getSortOrder()));
                         }
                     } else { // No nulls for fixed length
                         oStream.write(ptr.get(), ptr.getOffset(), ptr.getLength());
@@ -123,7 +124,7 @@ public class ArrayConstructorExpression extends BaseCompoundExpression {
             if (position >= 0) position = elements.length;
             if (!baseType.isFixedWidth()) {
                 // Double seperator byte to show end of the non null array
-                PArrayDataType.writeEndSeperatorForVarLengthArray(oStream, getSortOrder());
+                PArrayDataType.writeEndSeperatorForVarLengthArray(oStream, getSortOrder(), rowKeyOrderOptimizable);
                 noOfElements = PArrayDataType.serailizeOffsetArrayIntoStream(oStream, byteStream, noOfElements,
                         offsetPos[offsetPos.length - 1], offsetPos);
                 PArrayDataType.serializeHeaderInfoIntoStream(oStream, noOfElements);
@@ -147,14 +148,23 @@ public class ArrayConstructorExpression extends BaseCompoundExpression {
     @Override
     public void readFields(DataInput input) throws IOException {
         super.readFields(input);
+        boolean rowKeyOrderOptimizable = false;
         int baseTypeOrdinal = WritableUtils.readVInt(input);
-        init(PDataType.values()[baseTypeOrdinal]);
+        if (baseTypeOrdinal < 0) {
+            rowKeyOrderOptimizable = true;
+            baseTypeOrdinal = -(baseTypeOrdinal+1);
+        }
+        init(PDataType.values()[baseTypeOrdinal], rowKeyOrderOptimizable);
     }
 
     @Override
     public void write(DataOutput output) throws IOException {
         super.write(output);
-        WritableUtils.writeVInt(output, baseType.ordinal());
+        if (rowKeyOrderOptimizable) {
+            WritableUtils.writeVInt(output, -(baseType.ordinal()+1));
+        } else {
+            WritableUtils.writeVInt(output, baseType.ordinal());
+        }
     }
     
     @Override

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
index e4f53ae..efdceac 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/BaseExpression.java
@@ -33,12 +33,12 @@ import org.apache.phoenix.expression.function.FloorDateExpression;
 import org.apache.phoenix.expression.function.FloorDecimalExpression;
 import org.apache.phoenix.expression.function.TimeUnit;
 import org.apache.phoenix.expression.visitor.ExpressionVisitor;
-import org.apache.phoenix.schema.types.PDecimal;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.TypeMismatchException;
 import org.apache.phoenix.schema.types.PDataType;
+import org.apache.phoenix.schema.types.PDecimal;
 import org.apache.phoenix.schema.types.PTimestamp;
 import org.apache.phoenix.schema.types.PUnsignedTimestamp;
-import org.apache.phoenix.schema.SortOrder;
-import org.apache.phoenix.schema.TypeMismatchException;
 
 import com.google.common.collect.Lists;
 
@@ -53,7 +53,7 @@ import com.google.common.collect.Lists;
 public abstract class BaseExpression implements Expression {
     
     public static interface ExpressionComparabilityWrapper {
-        public Expression wrap(Expression lhs, Expression rhs) throws SQLException;
+        public Expression wrap(Expression lhs, Expression rhs, boolean rowKeyOrderOptimizable) throws SQLException;
     }
     
     /*
@@ -68,7 +68,7 @@ public abstract class BaseExpression implements Expression {
         WRAPPERS[CompareOp.LESS.ordinal()] = new ExpressionComparabilityWrapper() {
 
             @Override
-            public Expression wrap(Expression lhs, Expression rhs) throws SQLException {
+            public Expression wrap(Expression lhs, Expression rhs, boolean rowKeyOrderOptimizable) throws SQLException {
                 Expression e = rhs;
                 PDataType rhsType = rhs.getDataType();
                 PDataType lhsType = lhs.getDataType();
@@ -77,7 +77,7 @@ public abstract class BaseExpression implements Expression {
                 } else if ((rhsType == PTimestamp.INSTANCE || rhsType == PUnsignedTimestamp.INSTANCE)  && (lhsType != PTimestamp.INSTANCE && lhsType != PUnsignedTimestamp.INSTANCE)) {
                     e = FloorDateExpression.create(rhs, TimeUnit.MILLISECOND);
                 }
-                e = CoerceExpression.create(e, lhsType, lhs.getSortOrder(), lhs.getMaxLength());
+                e = CoerceExpression.create(e, lhsType, lhs.getSortOrder(), lhs.getMaxLength(), rowKeyOrderOptimizable);
                 return e;
             }
             
@@ -87,7 +87,7 @@ public abstract class BaseExpression implements Expression {
         WRAPPERS[CompareOp.GREATER.ordinal()] = new ExpressionComparabilityWrapper() {
 
             @Override
-            public Expression wrap(Expression lhs, Expression rhs) throws SQLException {
+            public Expression wrap(Expression lhs, Expression rhs, boolean rowKeyOrderOptimizable) throws SQLException {
                 Expression e = rhs;
                 PDataType rhsType = rhs.getDataType();
                 PDataType lhsType = lhs.getDataType();
@@ -96,7 +96,7 @@ public abstract class BaseExpression implements Expression {
                 } else if ((rhsType == PTimestamp.INSTANCE || rhsType == PUnsignedTimestamp.INSTANCE)  && (lhsType != PTimestamp.INSTANCE && lhsType != PUnsignedTimestamp.INSTANCE)) {
                     e = CeilTimestampExpression.create(rhs);
                 }
-                e = CoerceExpression.create(e, lhsType, lhs.getSortOrder(), lhs.getMaxLength());
+                e = CoerceExpression.create(e, lhsType, lhs.getSortOrder(), lhs.getMaxLength(), rowKeyOrderOptimizable);
                 return e;
             }
             
@@ -105,9 +105,9 @@ public abstract class BaseExpression implements Expression {
         WRAPPERS[CompareOp.EQUAL.ordinal()] = new ExpressionComparabilityWrapper() {
 
             @Override
-            public Expression wrap(Expression lhs, Expression rhs) throws SQLException {
+            public Expression wrap(Expression lhs, Expression rhs, boolean rowKeyOrderOptimizable) throws SQLException {
                 PDataType lhsType = lhs.getDataType();
-                Expression e = CoerceExpression.create(rhs, lhsType, lhs.getSortOrder(), lhs.getMaxLength());
+                Expression e = CoerceExpression.create(rhs, lhsType, lhs.getSortOrder(), lhs.getMaxLength(), rowKeyOrderOptimizable);
                 return e;
             }
             
@@ -127,42 +127,43 @@ public abstract class BaseExpression implements Expression {
      * @param lhs left hand side expression
      * @param rhs right hand side expression
      * @param op operator being used to compare the expressions, which can affect rounding we may need to do.
+     * @param rowKeyOrderOptimizable 
      * @return the newly coerced expression
      * @throws SQLException
      */
-    public static Expression coerce(Expression lhs, Expression rhs, CompareOp op) throws SQLException {
-        return coerce(lhs, rhs, getWrapper(op));
+    public static Expression coerce(Expression lhs, Expression rhs, CompareOp op, boolean rowKeyOrderOptimizable) throws SQLException {
+        return coerce(lhs, rhs, getWrapper(op), rowKeyOrderOptimizable);
     }
         
-    public static Expression coerce(Expression lhs, Expression rhs, ExpressionComparabilityWrapper wrapper) throws SQLException {
+    public static Expression coerce(Expression lhs, Expression rhs, ExpressionComparabilityWrapper wrapper, boolean rowKeyOrderOptimizable) throws SQLException {
         
         if (lhs instanceof RowValueConstructorExpression && rhs instanceof RowValueConstructorExpression) {
             int i = 0;
             List<Expression> coercedNodes = Lists.newArrayListWithExpectedSize(Math.max(lhs.getChildren().size(), rhs.getChildren().size()));
             for (; i < Math.min(lhs.getChildren().size(),rhs.getChildren().size()); i++) {
-                coercedNodes.add(coerce(lhs.getChildren().get(i), rhs.getChildren().get(i), wrapper));
+                coercedNodes.add(coerce(lhs.getChildren().get(i), rhs.getChildren().get(i), wrapper, rowKeyOrderOptimizable));
             }
             for (; i < lhs.getChildren().size(); i++) {
-                coercedNodes.add(coerce(lhs.getChildren().get(i), null, wrapper));
+                coercedNodes.add(coerce(lhs.getChildren().get(i), null, wrapper, rowKeyOrderOptimizable));
             }
             for (; i < rhs.getChildren().size(); i++) {
-                coercedNodes.add(coerce(null, rhs.getChildren().get(i), wrapper));
+                coercedNodes.add(coerce(null, rhs.getChildren().get(i), wrapper, rowKeyOrderOptimizable));
             }
             trimTrailingNulls(coercedNodes);
             return coercedNodes.equals(rhs.getChildren()) ? rhs : new RowValueConstructorExpression(coercedNodes, rhs.isStateless());
         } else if (lhs instanceof RowValueConstructorExpression) {
             List<Expression> coercedNodes = Lists.newArrayListWithExpectedSize(Math.max(rhs.getChildren().size(), lhs.getChildren().size()));
-            coercedNodes.add(coerce(lhs.getChildren().get(0), rhs, wrapper));
+            coercedNodes.add(coerce(lhs.getChildren().get(0), rhs, wrapper, rowKeyOrderOptimizable));
             for (int i = 1; i < lhs.getChildren().size(); i++) {
-                coercedNodes.add(coerce(lhs.getChildren().get(i), null, wrapper));
+                coercedNodes.add(coerce(lhs.getChildren().get(i), null, wrapper, rowKeyOrderOptimizable));
             }
             trimTrailingNulls(coercedNodes);
             return coercedNodes.equals(rhs.getChildren()) ? rhs : new RowValueConstructorExpression(coercedNodes, rhs.isStateless());
         } else if (rhs instanceof RowValueConstructorExpression) {
             List<Expression> coercedNodes = Lists.newArrayListWithExpectedSize(Math.max(rhs.getChildren().size(), lhs.getChildren().size()));
-            coercedNodes.add(coerce(lhs, rhs.getChildren().get(0), wrapper));
+            coercedNodes.add(coerce(lhs, rhs.getChildren().get(0), wrapper, rowKeyOrderOptimizable));
             for (int i = 1; i < rhs.getChildren().size(); i++) {
-                coercedNodes.add(coerce(null, rhs.getChildren().get(i), wrapper));
+                coercedNodes.add(coerce(null, rhs.getChildren().get(i), wrapper, rowKeyOrderOptimizable));
             }
             trimTrailingNulls(coercedNodes);
             return coercedNodes.equals(rhs.getChildren()) ? rhs : new RowValueConstructorExpression(coercedNodes, rhs.isStateless());
@@ -174,7 +175,7 @@ public abstract class BaseExpression implements Expression {
             if (rhs.getDataType() != null && lhs.getDataType() != null && !rhs.getDataType().isCastableTo(lhs.getDataType())) {
                 throw TypeMismatchException.newException(lhs.getDataType(), rhs.getDataType());
             }
-            return wrapper.wrap(lhs, rhs);
+            return wrapper.wrap(lhs, rhs, rowKeyOrderOptimizable);
         }
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/CoerceExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/CoerceExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/CoerceExpression.java
index b0396e8..fb6a825 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/CoerceExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/CoerceExpression.java
@@ -38,6 +38,7 @@ public class CoerceExpression extends BaseSingleExpression {
     private PDataType toType;
     private SortOrder toSortOrder;
     private Integer maxLength;
+    private boolean rowKeyOrderOptimizable;
     
     public CoerceExpression() {
     }
@@ -49,32 +50,33 @@ public class CoerceExpression extends BaseSingleExpression {
         return new CoerceExpression(expression, toType);
     }
     
-    public static Expression create(Expression expression, PDataType toType, SortOrder toSortOrder, Integer maxLength) throws SQLException {
+    public static Expression create(Expression expression, PDataType toType, SortOrder toSortOrder, Integer maxLength, boolean rowKeyOrderOptimizable) throws SQLException {
         if (toType == expression.getDataType() && toSortOrder == expression.getSortOrder()) {
             return expression;
         }
-        return new CoerceExpression(expression, toType, toSortOrder, maxLength);
+        return new CoerceExpression(expression, toType, toSortOrder, maxLength, rowKeyOrderOptimizable);
     }
     
     //Package protected for tests
     CoerceExpression(Expression expression, PDataType toType) {
-        this(expression, toType, SortOrder.getDefault(), null);
+        this(expression, toType, SortOrder.getDefault(), null, true);
     }
     
-    CoerceExpression(Expression expression, PDataType toType, SortOrder toSortOrder, Integer maxLength) {
-        this(ImmutableList.of(expression), toType, toSortOrder, maxLength);
+    CoerceExpression(Expression expression, PDataType toType, SortOrder toSortOrder, Integer maxLength, boolean rowKeyOrderOptimizable) {
+        this(ImmutableList.of(expression), toType, toSortOrder, maxLength, rowKeyOrderOptimizable);
     }
 
-    public CoerceExpression(List<Expression> children, PDataType toType, SortOrder toSortOrder, Integer maxLength) {
+    public CoerceExpression(List<Expression> children, PDataType toType, SortOrder toSortOrder, Integer maxLength, boolean rowKeyOrderOptimizable) {
         super(children);
         Preconditions.checkNotNull(toSortOrder);
         this.toType = toType;
         this.toSortOrder = toSortOrder;
         this.maxLength = maxLength;
+        this.rowKeyOrderOptimizable = rowKeyOrderOptimizable;
     }
     
     public CoerceExpression clone(List<Expression> children) {
-        return new CoerceExpression(children, this.getDataType(), this.getSortOrder(), this.getMaxLength());
+        return new CoerceExpression(children, this.getDataType(), this.getSortOrder(), this.getMaxLength(), this.rowKeyOrderOptimizable);
     }
     
     @Override
@@ -105,13 +107,19 @@ public class CoerceExpression extends BaseSingleExpression {
         if (toType == null) {
             if (other.toType != null) return false;
         } else if (!toType.equals(other.toType)) return false;
-        return true;
+        return rowKeyOrderOptimizable == other.rowKeyOrderOptimizable;
     }
 
     @Override
     public void readFields(DataInput input) throws IOException {
         super.readFields(input);
-        toType = PDataType.values()[WritableUtils.readVInt(input)];
+        int ordinal = WritableUtils.readVInt(input);
+        rowKeyOrderOptimizable = false;
+        if (ordinal < 0) {
+            rowKeyOrderOptimizable = true;
+            ordinal = -(ordinal+1);
+        }
+        toType = PDataType.values()[ordinal];
         toSortOrder = SortOrder.fromSystemValue(WritableUtils.readVInt(input));
         int byteSize = WritableUtils.readVInt(input);
         this.maxLength = byteSize == -1 ? null : byteSize;
@@ -120,16 +128,21 @@ public class CoerceExpression extends BaseSingleExpression {
     @Override
     public void write(DataOutput output) throws IOException {
         super.write(output);
-        WritableUtils.writeVInt(output, toType.ordinal());
+        if (rowKeyOrderOptimizable) {
+            WritableUtils.writeVInt(output, -(toType.ordinal()+1));
+        } else {
+            WritableUtils.writeVInt(output, toType.ordinal());
+        }
         WritableUtils.writeVInt(output, toSortOrder.getSystemValue());
         WritableUtils.writeVInt(output, maxLength == null ? -1 : maxLength);
     }
-    
+
     @Override
     public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
         if (getChild().evaluate(tuple, ptr)) {
-            getDataType().coerceBytes(ptr, getChild().getDataType(), getChild().getSortOrder(), getSortOrder(),
-                    getChild().getMaxLength());
+            getDataType().coerceBytes(ptr, null, getChild().getDataType(),
+                    getChild().getMaxLength(), null, getChild().getSortOrder(), 
+                    maxLength, null, getSortOrder(), rowKeyOrderOptimizable);
             return true;
         }
         return false;

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
index b9190e2..1c8df20 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
@@ -59,7 +59,7 @@ import com.google.common.collect.Lists;
 public class ComparisonExpression extends BaseCompoundExpression {
     private CompareOp op;
     
-    private static void addEqualityExpression(Expression lhs, Expression rhs, List<Expression> andNodes, ImmutableBytesWritable ptr) throws SQLException {
+    private static void addEqualityExpression(Expression lhs, Expression rhs, List<Expression> andNodes, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
         boolean isLHSNull = ExpressionUtil.isNull(lhs, ptr);
         boolean isRHSNull = ExpressionUtil.isNull(rhs, ptr);
         if (isLHSNull && isRHSNull) { // null == null will end up making the query degenerate
@@ -69,7 +69,7 @@ public class ComparisonExpression extends BaseCompoundExpression {
         } else if (isRHSNull) { // AND lhs IS NULL
             andNodes.add(IsNullExpression.create(lhs, false, ptr));
         } else { // AND lhs = rhs
-            andNodes.add(ComparisonExpression.create(CompareOp.EQUAL, Arrays.asList(lhs, rhs), ptr));
+            andNodes.add(ComparisonExpression.create(CompareOp.EQUAL, Arrays.asList(lhs, rhs), ptr, rowKeyOrderOptimizable));
         }
     }
     
@@ -81,32 +81,32 @@ public class ComparisonExpression extends BaseCompoundExpression {
      * @param andNodes
      * @throws SQLException 
      */
-    private static void rewriteRVCAsEqualityExpression(Expression lhs, Expression rhs, List<Expression> andNodes, ImmutableBytesWritable ptr) throws SQLException {
+    private static void rewriteRVCAsEqualityExpression(Expression lhs, Expression rhs, List<Expression> andNodes, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
         if (lhs instanceof RowValueConstructorExpression && rhs instanceof RowValueConstructorExpression) {
             int i = 0;
             for (; i < Math.min(lhs.getChildren().size(),rhs.getChildren().size()); i++) {
-                addEqualityExpression(lhs.getChildren().get(i), rhs.getChildren().get(i), andNodes, ptr);
+                addEqualityExpression(lhs.getChildren().get(i), rhs.getChildren().get(i), andNodes, ptr, rowKeyOrderOptimizable);
             }
             for (; i < lhs.getChildren().size(); i++) {
-                addEqualityExpression(lhs.getChildren().get(i), LiteralExpression.newConstant(null, lhs.getChildren().get(i).getDataType()), andNodes, ptr);
+                addEqualityExpression(lhs.getChildren().get(i), LiteralExpression.newConstant(null, lhs.getChildren().get(i).getDataType()), andNodes, ptr, rowKeyOrderOptimizable);
             }
             for (; i < rhs.getChildren().size(); i++) {
-                addEqualityExpression(LiteralExpression.newConstant(null, rhs.getChildren().get(i).getDataType()), rhs.getChildren().get(i), andNodes, ptr);
+                addEqualityExpression(LiteralExpression.newConstant(null, rhs.getChildren().get(i).getDataType()), rhs.getChildren().get(i), andNodes, ptr, rowKeyOrderOptimizable);
             }
         } else if (lhs instanceof RowValueConstructorExpression) {
-            addEqualityExpression(lhs.getChildren().get(0), rhs, andNodes, ptr);
+            addEqualityExpression(lhs.getChildren().get(0), rhs, andNodes, ptr, rowKeyOrderOptimizable);
             for (int i = 1; i < lhs.getChildren().size(); i++) {
-                addEqualityExpression(lhs.getChildren().get(i), LiteralExpression.newConstant(null, lhs.getChildren().get(i).getDataType()), andNodes, ptr);
+                addEqualityExpression(lhs.getChildren().get(i), LiteralExpression.newConstant(null, lhs.getChildren().get(i).getDataType()), andNodes, ptr, rowKeyOrderOptimizable);
             }
         } else if (rhs instanceof RowValueConstructorExpression) {
-            addEqualityExpression(lhs, rhs.getChildren().get(0), andNodes, ptr);
+            addEqualityExpression(lhs, rhs.getChildren().get(0), andNodes, ptr, rowKeyOrderOptimizable);
             for (int i = 1; i < rhs.getChildren().size(); i++) {
-                addEqualityExpression(LiteralExpression.newConstant(null, rhs.getChildren().get(i).getDataType()), rhs.getChildren().get(i), andNodes, ptr);
+                addEqualityExpression(LiteralExpression.newConstant(null, rhs.getChildren().get(i).getDataType()), rhs.getChildren().get(i), andNodes, ptr, rowKeyOrderOptimizable);
             }
         }
     }
     
-    public static Expression create(CompareOp op, List<Expression> children, ImmutableBytesWritable ptr) throws SQLException {
+    public static Expression create(CompareOp op, List<Expression> children, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
         Expression lhsExpr = children.get(0);
         Expression rhsExpr = children.get(1);
         PDataType lhsExprDataType = lhsExpr.getDataType();
@@ -115,14 +115,14 @@ public class ComparisonExpression extends BaseCompoundExpression {
         if ((lhsExpr instanceof RowValueConstructorExpression || rhsExpr instanceof RowValueConstructorExpression) && !(lhsExpr instanceof ArrayElemRefExpression) && !(rhsExpr instanceof ArrayElemRefExpression)) {
             if (op == CompareOp.EQUAL || op == CompareOp.NOT_EQUAL) {
                 List<Expression> andNodes = Lists.<Expression>newArrayListWithExpectedSize(Math.max(lhsExpr.getChildren().size(), rhsExpr.getChildren().size()));
-                rewriteRVCAsEqualityExpression(lhsExpr, rhsExpr, andNodes, ptr);
+                rewriteRVCAsEqualityExpression(lhsExpr, rhsExpr, andNodes, ptr, rowKeyOrderOptimizable);
                 Expression expr = AndExpression.create(andNodes);
                 if (op == CompareOp.NOT_EQUAL) {
                     expr = NotExpression.create(expr, ptr);
                 }
                 return expr;
             }
-            rhsExpr = RowValueConstructorExpression.coerce(lhsExpr, rhsExpr, op);
+            rhsExpr = RowValueConstructorExpression.coerce(lhsExpr, rhsExpr, op, rowKeyOrderOptimizable);
             // Always wrap both sides in row value constructor, so we don't have to consider comparing
             // a non rvc with a rvc.
             if ( ! ( lhsExpr instanceof RowValueConstructorExpression ) ) {
@@ -164,11 +164,13 @@ public class ComparisonExpression extends BaseCompoundExpression {
             // into account the comparison operator.
             if (rhsExprDataType != lhsExprDataType 
                     || rhsExpr.getSortOrder() != lhsExpr.getSortOrder()
-                    || (rhsExprDataType.isFixedWidth() && rhsExpr.getMaxLength() != null && lhsExprDataType.isFixedWidth() && lhsExpr.getMaxLength() != null && rhsExpr.getMaxLength() < lhsExpr.getMaxLength())) {
+                    || (rhsExprDataType.isFixedWidth() && rhsExpr.getMaxLength() != null && 
+                        lhsExprDataType.isFixedWidth() && lhsExpr.getMaxLength() != null && 
+                        rhsExpr.getMaxLength() < lhsExpr.getMaxLength())) {
                 // TODO: if lengths are unequal and fixed width?
                 if (rhsExprDataType.isCoercibleTo(lhsExprDataType, rhsValue)) { // will convert 2.0 -> 2
                     children = Arrays.asList(children.get(0), LiteralExpression.newConstant(rhsValue, lhsExprDataType, 
-                            lhsExpr.getMaxLength(), null, lhsExpr.getSortOrder(), determinism));
+                            lhsExpr.getMaxLength(), null, lhsExpr.getSortOrder(), determinism, rowKeyOrderOptimizable));
                 } else if (op == CompareOp.EQUAL) {
                     return LiteralExpression.newConstant(false, PBoolean.INSTANCE, Determinism.ALWAYS);
                 } else if (op == CompareOp.NOT_EQUAL) {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
index 63178db..b6d5a24 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/InListExpression.java
@@ -39,6 +39,7 @@ import org.apache.phoenix.schema.types.PBoolean;
 import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.ExpressionUtil;
+import org.apache.phoenix.util.StringUtil;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -56,15 +57,17 @@ public class InListExpression extends BaseSingleExpression {
     private int valuesByteLength;
     private int fixedWidth = -1;
     private List<Expression> keyExpressions; // client side only
+    private boolean rowKeyOrderOptimizable; // client side only
 
-    public static Expression create (List<Expression> children, boolean isNegate, ImmutableBytesWritable ptr) throws SQLException {
+
+    public static Expression create (List<Expression> children, boolean isNegate, ImmutableBytesWritable ptr, boolean rowKeyOrderOptimizable) throws SQLException {
         Expression firstChild = children.get(0);
         
         if (firstChild.isStateless() && (!firstChild.evaluate(null, ptr) || ptr.getLength() == 0)) {
             return LiteralExpression.newConstant(null, PBoolean.INSTANCE, firstChild.getDeterminism());
         }
         if (children.size() == 2) {
-            return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL, children, ptr);
+            return ComparisonExpression.create(isNegate ? CompareOp.NOT_EQUAL : CompareOp.EQUAL, children, ptr, rowKeyOrderOptimizable);
         }
         
         boolean addedNull = false;
@@ -73,7 +76,7 @@ public class InListExpression extends BaseSingleExpression {
         coercedKeyExpressions.add(firstChild);
         for (int i = 1; i < children.size(); i++) {
             try {
-                Expression rhs = BaseExpression.coerce(firstChild, children.get(i), CompareOp.EQUAL);
+                Expression rhs = BaseExpression.coerce(firstChild, children.get(i), CompareOp.EQUAL, rowKeyOrderOptimizable);
                 coercedKeyExpressions.add(rhs);
             } catch (SQLException e) {
                 // Type mismatch exception or invalid data exception.
@@ -89,7 +92,7 @@ public class InListExpression extends BaseSingleExpression {
         if (coercedKeyExpressions.size() == 2 && addedNull) {
             return LiteralExpression.newConstant(null, PBoolean.INSTANCE, Determinism.ALWAYS);
         }
-        Expression expression = new InListExpression(coercedKeyExpressions);
+        Expression expression = new InListExpression(coercedKeyExpressions, rowKeyOrderOptimizable);
         if (isNegate) { 
             expression = NotExpression.create(expression, ptr);
         }
@@ -102,10 +105,13 @@ public class InListExpression extends BaseSingleExpression {
     public InListExpression() {
     }
 
-    public InListExpression(List<Expression> keyExpressions) {
+    public InListExpression(List<Expression> keyExpressions, boolean rowKeyOrderOptimizable) {
         super(keyExpressions.get(0));
+        this.rowKeyOrderOptimizable = rowKeyOrderOptimizable;
+        Expression firstChild = keyExpressions.get(0);
         this.keyExpressions = keyExpressions.subList(1, keyExpressions.size());
         Set<ImmutableBytesPtr> values = Sets.newHashSetWithExpectedSize(keyExpressions.size()-1);
+        Integer maxLength = firstChild.getDataType().isFixedWidth() ? firstChild.getMaxLength() : null;
         int fixedWidth = -1;
         boolean isFixedLength = true;
         for (int i = 1; i < keyExpressions.size(); i++) {
@@ -113,6 +119,12 @@ public class InListExpression extends BaseSingleExpression {
             Expression child = keyExpressions.get(i);
             child.evaluate(null, ptr);
             if (ptr.getLength() > 0) { // filter null as it has no impact
+                if (rowKeyOrderOptimizable) {
+                    firstChild.getDataType().pad(ptr, maxLength, firstChild.getSortOrder());
+                } else if (maxLength != null) {
+                    byte[] paddedBytes = StringUtil.padChar(ByteUtil.copyKeyBytesIfNecessary(ptr), maxLength);
+                    ptr.set(paddedBytes);
+                }
                 if (values.add(ptr)) {
                     int length = ptr.getLength();
                     if (fixedWidth == -1) {
@@ -274,4 +286,8 @@ public class InListExpression extends BaseSingleExpression {
         buf.setCharAt(buf.length()-1,')');
         return buf.toString();
     }
+
+    public InListExpression clone(List<Expression> l) {
+        return new InListExpression(l, this.rowKeyOrderOptimizable);
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java
index c1faf66..05bd9b3 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java
@@ -28,6 +28,7 @@ import org.apache.phoenix.expression.visitor.ExpressionVisitor;
 import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.TypeMismatchException;
 import org.apache.phoenix.schema.tuple.Tuple;
+import org.apache.phoenix.schema.types.PArrayDataType;
 import org.apache.phoenix.schema.types.PBoolean;
 import org.apache.phoenix.schema.types.PChar;
 import org.apache.phoenix.schema.types.PDataType;
@@ -37,6 +38,7 @@ import org.apache.phoenix.schema.types.PTimestamp;
 import org.apache.phoenix.schema.types.PVarchar;
 import org.apache.phoenix.schema.types.PhoenixArray;
 import org.apache.phoenix.util.ByteUtil;
+import org.apache.phoenix.util.StringUtil;
 
 import com.google.common.base.Preconditions;
 
@@ -150,8 +152,13 @@ public class LiteralExpression extends BaseTerminalExpression {
         return newConstant(value, type, maxLength, scale, SortOrder.getDefault(), determinism);
     }
 
+    public static LiteralExpression newConstant(Object value, PDataType type, Integer maxLength, Integer scale, SortOrder sortOrder, Determinism determinism) 
+            throws SQLException {
+        return newConstant(value, type, maxLength, scale, sortOrder, determinism, true);
+    }
+    
     // TODO: cache?
-    public static LiteralExpression newConstant(Object value, PDataType type, Integer maxLength, Integer scale, SortOrder sortOrder, Determinism determinism)
+    public static LiteralExpression newConstant(Object value, PDataType type, Integer maxLength, Integer scale, SortOrder sortOrder, Determinism determinism, boolean rowKeyOrderOptimizable)
             throws SQLException {
         if (value == null) {
             return  (type == null) ?  getNullLiteralExpression(determinism) : getTypedNullLiteralExpression(type, determinism);
@@ -170,10 +177,15 @@ public class LiteralExpression extends BaseTerminalExpression {
             throw TypeMismatchException.newException(type, actualType, value.toString());
         }
         value = type.toObject(value, actualType);
-        byte[] b = type.toBytes(value, sortOrder);
+        byte[] b = type.isArrayType() ? ((PArrayDataType)type).toBytes(value, PArrayDataType.arrayBaseType(type), sortOrder, rowKeyOrderOptimizable) :
+                type.toBytes(value, sortOrder);
         if (type == PVarchar.INSTANCE || type == PChar.INSTANCE) {
             if (type == PChar.INSTANCE && maxLength != null  && b.length < maxLength) {
-                b = type.pad(b, maxLength, sortOrder);
+                if (rowKeyOrderOptimizable) {
+                    b = type.pad(b, maxLength, sortOrder);
+                } else {
+                    b = StringUtil.padChar(b, maxLength);
+                }
             } else if (value != null) {
                 maxLength = ((String)value).length();
             }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java
index 6a3e2a1..3615cbe 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/InvertFunction.java
@@ -47,9 +47,9 @@ public class InvertFunction extends ScalarFunction {
     public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
         if (!getChildExpression().evaluate(tuple, ptr)) { return false; }
         if (ptr.getLength() == 0) { return true; }
-        byte[] buf = new byte[ptr.getLength()];
-        SortOrder.invert(ptr.get(), ptr.getOffset(), buf, 0, ptr.getLength());
-        ptr.set(buf);
+        PDataType type = getDataType();
+        // FIXME: losing rowKeyOrderOptimizable here
+        type.coerceBytes(ptr, type, getChildExpression().getSortOrder(), getSortOrder());
         return true;
     }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java
index 111c8b3..cb98e28 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/PrefixFunction.java
@@ -18,6 +18,7 @@
 
 package org.apache.phoenix.expression.function;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -25,6 +26,7 @@ import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
 import org.apache.phoenix.compile.KeyPart;
 import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.PColumn;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.SortOrder;
@@ -84,14 +86,28 @@ abstract public class PrefixFunction extends ScalarFunction {
                 default:
                     return childPart.getKeyRange(op, rhs);
                 }
-                Integer length = getColumn().getMaxLength();
-                SortOrder sortOrder = getColumn().getSortOrder();
-                if (type.isFixedWidth() && length != null) {
-                    if (lowerRange != KeyRange.UNBOUND) {
-                        lowerRange = type.pad(lowerRange, length, sortOrder);
+                PColumn column = getColumn();
+                Integer length = column.getMaxLength();
+                if (type.isFixedWidth()) {
+                    if (length != null) { // Sanity check - shouldn't be necessary
+                        // Don't pad based on current sort order, but instead use our
+                        // minimum byte as otherwise we'll end up skipping rows in
+                        // the case of descending, since rows with more padding appear
+                        // *after* rows with no padding.
+                        if (lowerRange != KeyRange.UNBOUND) {
+                            lowerRange = type.pad(lowerRange, length, SortOrder.ASC);
+                        }
+                        if (upperRange != KeyRange.UNBOUND) {
+                            upperRange = type.pad(upperRange, length, SortOrder.ASC);
+                        }
                     }
-                    if (upperRange != KeyRange.UNBOUND) {
-                        upperRange = type.pad(upperRange, length, sortOrder);
+                } else if (column.getSortOrder() == SortOrder.DESC && getTable().rowKeyOrderOptimizable()) {
+                    // Append a zero byte if descending since a \xFF byte will be appended to the lowerRange
+                    // causing rows to be skipped that should be included. For example, with rows 'ab', 'a',
+                    // a lowerRange of 'a\xFF' would skip 'ab', while 'a\x00\xFF' would not.
+                    if (lowerRange != KeyRange.UNBOUND) {
+                        lowerRange = Arrays.copyOf(lowerRange, lowerRange.length+1);
+                        lowerRange[lowerRange.length-1] = QueryConstants.SEPARATOR_BYTE;
                     }
                 }
                 return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, false);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java
index aa632e2..5713713 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/RTrimFunction.java
@@ -18,6 +18,7 @@
 package org.apache.phoenix.expression.function;
 
 import java.sql.SQLException;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -28,6 +29,7 @@ import org.apache.phoenix.expression.Expression;
 import org.apache.phoenix.parse.FunctionParseNode.Argument;
 import org.apache.phoenix.parse.FunctionParseNode.BuiltInFunction;
 import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.PColumn;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.SortOrder;
@@ -109,35 +111,70 @@ public class RTrimFunction extends ScalarFunction {
                 byte[] lowerRange = KeyRange.UNBOUND;
                 byte[] upperRange = KeyRange.UNBOUND;
                 boolean lowerInclusive = true;
+                boolean upperInclusive = false;
                 
                 PDataType type = getColumn().getDataType();
+                SortOrder sortOrder = getColumn().getSortOrder();
                 switch (op) {
-                case EQUAL:
-                    lowerRange = evaluateExpression(rhs);
-                    upperRange = ByteUtil.nextKey(ByteUtil.concat(lowerRange, new byte[] {StringUtil.SPACE_UTF8}));
-                    break;
                 case LESS_OR_EQUAL:
                     lowerInclusive = false;
-                    upperRange = ByteUtil.nextKey(ByteUtil.concat(evaluateExpression(rhs), new byte[] {StringUtil.SPACE_UTF8}));
+                case EQUAL:
+                    upperRange = evaluateExpression(rhs);
+                    if (op == CompareOp.EQUAL) {
+                        lowerRange = upperRange;
+                    }
+                    if (sortOrder == SortOrder.ASC || !getTable().rowKeyOrderOptimizable()) {
+                        upperRange = Arrays.copyOf(upperRange, upperRange.length + 1);
+                        upperRange[upperRange.length-1] = StringUtil.SPACE_UTF8;
+                        ByteUtil.nextKey(upperRange, upperRange.length);
+                    } else {
+                        upperInclusive = true;
+                        if (op == CompareOp.LESS_OR_EQUAL) {
+                            // Nothing more to do here, as the biggest value for DESC
+                            // will be the RHS value.
+                            break;
+                        }
+                        /*
+                         * Somewhat tricky to get the range correct for the DESC equality case.
+                         * The lower range is the RHS value followed by any number of inverted spaces.
+                         * We need to add a zero byte as the lower range will have an \xFF byte
+                         * appended to it and otherwise we'd skip past any rows where there is more
+                         * than one space following the RHS.
+                         * The upper range should span up to and including the RHS value. We need
+                         * to add our own \xFF as otherwise this will look like a degenerate query
+                         * since the lower would be bigger than the upper range.
+                         */
+                        lowerRange = Arrays.copyOf(lowerRange, lowerRange.length + 2);
+                        lowerRange[lowerRange.length-2] = StringUtil.INVERTED_SPACE_UTF8;
+                        lowerRange[lowerRange.length-1] = QueryConstants.SEPARATOR_BYTE;
+                        upperRange = Arrays.copyOf(upperRange, upperRange.length + 1);
+                        upperRange[upperRange.length-1] = QueryConstants.DESC_SEPARATOR_BYTE;
+                    }
                     break;
                 default:
+                    // TOOD: Is this ok for DESC?
                     return childPart.getKeyRange(op, rhs);
                 }
                 Integer length = getColumn().getMaxLength();
-                SortOrder sortOrder = getColumn().getSortOrder();
                 if (type.isFixedWidth() && length != null) {
+                    // Don't pad based on current sort order, but instead use our
+                    // minimum byte as otherwise we'll end up skipping rows in
+                    // the case of descending, since rows with more padding appear
+                    // *after* rows with no padding.
                     if (lowerRange != KeyRange.UNBOUND) {
-                        lowerRange = type.pad(lowerRange, length, sortOrder);
+                        lowerRange = type.pad(lowerRange, length, SortOrder.ASC);
                     }
                     if (upperRange != KeyRange.UNBOUND) {
-                        upperRange = type.pad(upperRange, length, sortOrder);
+                        upperRange = type.pad(upperRange, length, SortOrder.ASC);
                     }
                 }
-                return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, false);
+                return KeyRange.getKeyRange(lowerRange, lowerInclusive, upperRange, upperInclusive);
             }
 
             @Override
             public List<Expression> getExtractNodes() {
+                // We cannot extract the node, as we may have false positives with trailing
+                // non blank characters such as 'foo  bar' where the RHS constant is 'foo'.
                 return Collections.<Expression>emptyList();
             }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
index b252358..55f227f 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/visitor/CloneExpressionVisitor.java
@@ -136,7 +136,7 @@ public class CloneExpressionVisitor extends TraverseAllExpressionVisitor<Express
 
     @Override
     public Expression visitLeave(InListExpression node, List<Expression> l) {
-        return Determinism.PER_INVOCATION.compareTo(node.getDeterminism()) > 0 ? node :  new InListExpression(l);
+        return Determinism.PER_INVOCATION.compareTo(node.getDeterminism()) > 0 ? node :  node.clone(l);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java
index bad02e4..6236184 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java
@@ -79,7 +79,7 @@ public abstract class PArrayDataType<T> extends PDataType<T> {
         super(sqlTypeName, sqlType, clazz, codec, ordinal);
     }
 
-    private static byte getSeparatorByte(boolean rowKeyOrderOptimizable, SortOrder sortOrder) {
+    public static byte getSeparatorByte(boolean rowKeyOrderOptimizable, SortOrder sortOrder) {
         return SchemaUtil.getSeparatorByte(rowKeyOrderOptimizable, false, sortOrder);
     }
 
@@ -177,7 +177,7 @@ public abstract class PArrayDataType<T> extends PDataType<T> {
         writeEndSeperatorForVarLengthArray(oStream, sortOrder, true);
     }
     
-    private static void writeEndSeperatorForVarLengthArray(DataOutputStream oStream, SortOrder sortOrder, boolean rowKeyOrderOptimizable)
+    public static void writeEndSeperatorForVarLengthArray(DataOutputStream oStream, SortOrder sortOrder, boolean rowKeyOrderOptimizable)
             throws IOException {
         byte sepByte = getSeparatorByte(rowKeyOrderOptimizable, sortOrder);
         oStream.write(sepByte);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
index 3f09518..96b8ea2 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
@@ -1669,6 +1669,14 @@ public abstract class BaseTest {
         fail("Unable to find " + results + " in " + Arrays.asList(expectedResultsArray));
     }
 
+    protected void assertValueEqualsResultSet(ResultSet rs, List<Object> expectedResults) throws SQLException {
+        List<List<Object>> nestedExpectedResults = Lists.newArrayListWithExpectedSize(expectedResults.size());
+        for (Object expectedResult : expectedResults) {
+            nestedExpectedResults.add(Arrays.asList(expectedResult));
+        }
+        assertValuesEqualsResultSet(rs, nestedExpectedResults); 
+    }
+
     /**
      * Asserts that we find the expected values in the result set. We don't know the order, since we don't always
      * have an order by and we're going through indexes, but we assert that each expected result occurs once as

http://git-wip-us.apache.org/repos/asf/phoenix/blob/498cc552/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
index 66695f8..15e3583 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
@@ -321,7 +321,7 @@ public class TestUtil {
     }
 
     public static Expression in(Expression... expressions) throws SQLException {
-        return InListExpression.create(Arrays.asList(expressions), false, new ImmutableBytesWritable());
+        return InListExpression.create(Arrays.asList(expressions), false, new ImmutableBytesWritable(), true);
     }
 
     public static Expression in(Expression e, Object... literals) throws SQLException {
@@ -331,7 +331,7 @@ public class TestUtil {
         for (Object o : literals) {
             expressions.add(LiteralExpression.newConstant(o, childType));
         }
-        return InListExpression.create(expressions, false, new ImmutableBytesWritable());
+        return InListExpression.create(expressions, false, new ImmutableBytesWritable(), true);
     }
 
     public static void assertDegenerate(StatementContext context) {


Mime
View raw message