You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2014/02/26 10:37:41 UTC

git commit: PHOENIX-77 Indexes and salting on views is broken (JamesTaylor)

Repository: incubator-phoenix
Updated Branches:
  refs/heads/master 1e2df030c -> c439d3a0c


PHOENIX-77 Indexes and salting on views is broken (JamesTaylor)


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

Branch: refs/heads/master
Commit: c439d3a0c23cd15879a6f25c28dc154faaecba6a
Parents: 1e2df03
Author: James Taylor <ja...@apache.org>
Authored: Wed Feb 26 01:32:56 2014 -0800
Committer: James Taylor <ja...@apache.org>
Committed: Wed Feb 26 01:32:56 2014 -0800

----------------------------------------------------------------------
 .../apache/phoenix/compile/UpsertCompiler.java  |  13 +-
 .../apache/phoenix/jdbc/PhoenixConnection.java  |  11 +-
 .../apache/phoenix/schema/MetaDataClient.java   |  68 ++++++----
 .../org/apache/phoenix/schema/PDataType.java    |   9 ++
 .../apache/phoenix/end2end/BaseViewTest.java    | 133 +++++++++++++++++++
 .../apache/phoenix/end2end/SaltedViewTest.java  |  35 +++++
 .../org/apache/phoenix/end2end/ViewTest.java    |  51 ++-----
 7 files changed, 241 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
index 0795fde..a2c6496 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
@@ -304,19 +304,22 @@ public class UpsertCompiler {
                 }
                 i++;
             }
+            int hiddenOffset = posOffset;
             // Add tenant column directly, as we don't want to resolve it as this will fail
             if (isTenantSpecific) {
-                PColumn tenantColumn = table.getPKColumns().get(posOffset);
+                PColumn tenantColumn = table.getPKColumns().get(hiddenOffset);
                 columnIndexesToBe[i] = tenantColumn.getPosition();
-                pkColumnsSet.set(pkSlotIndexesToBe[i] = posOffset);
+                pkColumnsSet.set(pkSlotIndexesToBe[i] = hiddenOffset);
                 targetColumns.set(i, tenantColumn);
+                hiddenOffset++;
                 i++;
             }
             if (isSharedViewIndex) {
-                PColumn indexIdColumn = table.getPKColumns().get(posOffset+1);
+                PColumn indexIdColumn = table.getPKColumns().get(hiddenOffset);
                 columnIndexesToBe[i] = indexIdColumn.getPosition();
-                pkColumnsSet.set(pkSlotIndexesToBe[i] = posOffset+1);
+                pkColumnsSet.set(pkSlotIndexesToBe[i] = hiddenOffset);
                 targetColumns.set(i, indexIdColumn);
+                hiddenOffset++;
                 i++;
             }
             i = posOffset;
@@ -808,7 +811,7 @@ public class UpsertCompiler {
     }
     
     private static SelectStatement addTenantAndViewConstants(PTable table, SelectStatement select, String tenantId, Map<ColumnRef, byte[]> addViewColumns) {
-        if (tenantId == null && addViewColumns.isEmpty()) {
+        if ((!table.isMultiTenant() || tenantId == null) && table.getViewIndexId() == null && addViewColumns.isEmpty()) {
             return select;
         }
         List<AliasedNode> selectNodes = newArrayListWithCapacity(select.getSelect().size() + 1 + addViewColumns.size());

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
index e400314..9f94623 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
@@ -50,13 +50,11 @@ import java.util.concurrent.Executor;
 
 import javax.annotation.Nullable;
 
-import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import org.apache.phoenix.client.KeyValueBuilder;
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.exception.SQLExceptionInfo;
 import org.apache.phoenix.execute.MutationState;
+import org.apache.phoenix.expression.function.FunctionArgumentType;
 import org.apache.phoenix.jdbc.PhoenixStatement.PhoenixStatementParser;
 import org.apache.phoenix.query.ConnectionQueryServices;
 import org.apache.phoenix.query.DelegateConnectionQueryServices;
@@ -73,11 +71,16 @@ import org.apache.phoenix.schema.PName;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.util.DateUtil;
 import org.apache.phoenix.util.JDBCUtil;
+import org.apache.phoenix.util.NumberUtil;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.SQLCloseable;
 import org.apache.phoenix.util.SQLCloseables;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
 
 /**
  * 
@@ -153,10 +156,12 @@ public class PhoenixConnection implements Connection, org.apache.phoenix.jdbc.Jd
         this.tenantId = JDBCUtil.getTenantId(url, this.info);
         this.mutateBatchSize = JDBCUtil.getMutateBatchSize(url, this.info, services.getProps());
         datePattern = services.getProps().get(QueryServices.DATE_FORMAT_ATTRIB, DateUtil.DEFAULT_DATE_FORMAT);
+        String numberPattern = services.getProps().get(QueryServices.NUMBER_FORMAT_ATTRIB, NumberUtil.DEFAULT_NUMBER_FORMAT);
         int maxSize = services.getProps().getInt(QueryServices.MAX_MUTATION_SIZE_ATTRIB,QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE);
         Format dateTimeFormat = DateUtil.getDateFormatter(datePattern);
         formatters[PDataType.DATE.ordinal()] = dateTimeFormat;
         formatters[PDataType.TIME.ordinal()] = dateTimeFormat;
+        formatters[PDataType.DECIMAL.ordinal()] = FunctionArgumentType.NUMERIC.getFormatter(numberPattern);
         this.metaData = PMetaDataImpl.pruneMultiTenant(metaData);
         this.mutationState = new MutationState(maxSize, this);
         services.addConnection(this);

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index 374aa99..0e8555d 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -406,7 +406,7 @@ public class MetaDataClient {
     }
 
     public MutationState createTable(CreateTableStatement statement, byte[][] splits, PTable parent, String viewStatement, ViewType viewType, byte[][] viewColumnConstants) throws SQLException {
-        PTable table = createTableInternal(statement, splits, parent, viewStatement, viewType, viewColumnConstants);
+        PTable table = createTableInternal(statement, splits, parent, viewStatement, viewType, viewColumnConstants, null);
         if (table == null || table.getType() == PTableType.VIEW) {
             return new MutationState(0,connection);
         }
@@ -513,6 +513,8 @@ public class MetaDataClient {
         TableRef tableRef = null;
         PTable table = null;
         boolean retry = true;
+        Short viewIndexId = null;
+        boolean allocateViewIndexId = false;
         while (true) {
             try {
                 ColumnResolver resolver = FromCompiler.getResolver(statement, connection);
@@ -554,6 +556,7 @@ public class MetaDataClient {
                     columnDefs.add(FACTORY.columnDef(colName, dataType.getSqlTypeName(), col.isNullable(), col.getMaxLength(), col.getScale(), false, SortOrder.getDefault()));
                 }
                 if (dataTable.getType() == PTableType.VIEW && dataTable.getViewType() != ViewType.MAPPED) {
+                    allocateViewIndexId = true;
                     // Next add index ID column
                     PDataType dataType = MetaDataUtil.getViewIndexIdDataType();
                     ColumnName colName = ColumnName.caseSensitiveColumnName(MetaDataUtil.getViewIndexIdColumnName());
@@ -611,13 +614,38 @@ public class MetaDataClient {
                     }
                 }
                 
+                // Don't re-allocate viewIndexId on ConcurrentTableMutationException,
+                // as there's no need to burn another sequence value.
+                if (allocateViewIndexId && viewIndexId == null) { 
+                    Long scn = connection.getSCN();
+                    long timestamp = scn == null ? HConstants.LATEST_TIMESTAMP : scn;
+                    PName tenantId = connection.getTenantId();
+                    String tenantIdStr = tenantId == null ? null : connection.getTenantId().getString();
+                    PName physicalName = dataTable.getPhysicalName();
+                    SequenceKey key = MetaDataUtil.getViewIndexSequenceKey(tenantIdStr, physicalName);
+                    // Create at parent timestamp as we know that will be earlier than now
+                    // and earlier than any SCN if one is set.
+                    createSequence(key.getTenantId(), key.getSchemaName(), key.getSequenceName(), true, Short.MIN_VALUE, 1, 1, dataTable.getTimeStamp());
+                    long[] seqValues = new long[1];
+                    SQLException[] sqlExceptions = new SQLException[1];
+                    connection.getQueryServices().incrementSequences(Collections.singletonList(key), timestamp, seqValues, sqlExceptions);
+                    if (sqlExceptions[0] != null) {
+                        throw sqlExceptions[0];
+                    }
+                    long seqValue = seqValues[0];
+                    if (seqValue > Short.MAX_VALUE) {
+                        throw new SQLExceptionInfo.Builder(SQLExceptionCode.TOO_MANY_VIEW_INDEXES)
+                        .setSchemaName(SchemaUtil.getSchemaNameFromFullName(physicalName.getString())).setTableName(SchemaUtil.getTableNameFromFullName(physicalName.getString())).build().buildException();
+                    }
+                    viewIndexId = (short) seqValue;
+                }
                 // Set DEFAULT_COLUMN_FAMILY_NAME of index to match data table
                 // We need this in the props so that the correct column family is created
                 if (dataTable.getDefaultFamilyName() != null && dataTable.getType() != PTableType.VIEW) {
                     statement.getProps().put("", new Pair<String,Object>(DEFAULT_COLUMN_FAMILY_NAME,dataTable.getDefaultFamilyName().getString()));
                 }
                 CreateTableStatement tableStatement = FACTORY.createTable(indexTableName, statement.getProps(), columnDefs, pk, statement.getSplitNodes(), PTableType.INDEX, statement.ifNotExists(), null, null, statement.getBindCount());
-                table = createTableInternal(tableStatement, splits, dataTable, null, null, EMPTY_VIEW_CONSTANTS);
+                table = createTableInternal(tableStatement, splits, dataTable, null, null, EMPTY_VIEW_CONSTANTS, viewIndexId);
                 break;
             } catch (ConcurrentTableMutationException e) { // Can happen if parent data table changes while above is in progress
                 if (retry) {
@@ -685,7 +713,7 @@ public class MetaDataClient {
         return null;
     }
     
-    private PTable createTableInternal(CreateTableStatement statement, byte[][] splits, final PTable parent, String viewStatement, ViewType viewType, byte[][] viewColumnConstants) throws SQLException {
+    private PTable createTableInternal(CreateTableStatement statement, byte[][] splits, final PTable parent, String viewStatement, ViewType viewType, byte[][] viewColumnConstants, Short viewIndexId) throws SQLException {
         final PTableType tableType = statement.getTableType();
         boolean wasAutoCommit = connection.getAutoCommit();
         connection.rollback();
@@ -697,17 +725,15 @@ public class MetaDataClient {
             String schemaName = tableNameNode.getSchemaName();
             String tableName = tableNameNode.getTableName();
             String parentTableName = null;
-            Long scn = connection.getSCN();
-            long timestamp = scn == null ? HConstants.LATEST_TIMESTAMP : scn;
             PName tenantId = connection.getTenantId();
             String tenantIdStr = tenantId == null ? null : connection.getTenantId().getString();
             boolean isParentImmutableRows = false;
-            Short viewIndexId = null;
             boolean multiTenant = false;
             Integer saltBucketNum = null;
             String defaultFamilyName = null;
             boolean isImmutableRows = false;
             List<PName> physicalNames = Collections.emptyList();
+            boolean isSalted = false;
             if (parent != null && tableType == PTableType.INDEX) {
                 // Index on view
                 // TODO: Can we support a multi-tenant index directly on a multi-tenant
@@ -715,25 +741,8 @@ public class MetaDataClient {
                 // from the table to the index, though.
                 if (parent.getType() == PTableType.VIEW && parent.getViewType() != ViewType.MAPPED) {
                     PName physicalName = parent.getPhysicalName();
-                    SequenceKey key = MetaDataUtil.getViewIndexSequenceKey(tenantIdStr, physicalName);
-                    // Create at parent timestamp as we know that will be earlier than now
-                    // and earlier than any SCN if one is set.
-                    // TODO: If sequence already exists, then we don't need to ensure metadata is already created.
-                    // Set physicalName to null in this case?
-                    createSequence(key.getTenantId(), key.getSchemaName(), key.getSequenceName(), true, Short.MIN_VALUE, 1, 1, parent.getTimeStamp());
-                    long[] seqValues = new long[1];
-                    SQLException[] sqlExceptions = new SQLException[1];
-                    connection.getQueryServices().incrementSequences(Collections.singletonList(key), timestamp, seqValues, sqlExceptions);
-                    if (sqlExceptions[0] != null) {
-                        throw sqlExceptions[0];
-                    }
-                    long seqValue = seqValues[0];
-                    if (seqValue > Short.MAX_VALUE) {
-                        throw new SQLExceptionInfo.Builder(SQLExceptionCode.TOO_MANY_VIEW_INDEXES)
-                        .setSchemaName(SchemaUtil.getSchemaNameFromFullName(physicalName.getString())).setTableName(SchemaUtil.getTableNameFromFullName(physicalName.getString())).build().buildException();
-                    }
-                    viewIndexId = (short) seqValue;
                     saltBucketNum = parent.getBucketNum();
+                    isSalted = (saltBucketNum != null);
                     defaultFamilyName = parent.getDefaultFamilyName() == null ? null : parent.getDefaultFamilyName().getString();
                     // Set physical name of view index table
                     physicalNames = Collections.singletonList(PNameFactory.newName(MetaDataUtil.getViewIndexPhysicalName(physicalName.getBytes())));
@@ -791,10 +800,9 @@ public class MetaDataClient {
                 }
             }
             
-            boolean isSalted = false;
             // Although unusual, it's possible to set a mapped VIEW as having immutable rows.
             // This tells Phoenix that you're managing the index maintenance yourself.
-            if (tableType != PTableType.VIEW || viewType == ViewType.MAPPED) {
+            if (tableType != PTableType.INDEX && (tableType != PTableType.VIEW || viewType == ViewType.MAPPED)) {
                 Boolean isImmutableRowsProp = (Boolean) tableProps.remove(PTable.IS_IMMUTABLE_ROWS_PROP_NAME);
                 if (isImmutableRowsProp == null) {
                     isImmutableRows = connection.getQueryServices().getProps().getBoolean(QueryServices.IMMUTABLE_ROWS_ATTRIB, QueryServicesOptions.DEFAULT_IMMUTABLE_ROWS);
@@ -862,8 +870,12 @@ public class MetaDataClient {
                     isImmutableRows = parent.isImmutableRows();
                     disableWAL = (disableWALProp == null ? parent.isWALDisabled() : disableWALProp);
                     defaultFamilyName = parent.getDefaultFamilyName() == null ? null : parent.getDefaultFamilyName().getString();
-                    columns = newArrayListWithExpectedSize(parent.getColumns().size() + colDefs.size());
-                    columns.addAll(parent.getColumns());
+                    List<PColumn> allColumns = parent.getColumns();
+                    if (saltBucketNum != null) { // Don't include salt column in columns, as it should not have it when created
+                        allColumns = allColumns.subList(1, allColumns.size());
+                    }
+                    columns = newArrayListWithExpectedSize(allColumns.size() + colDefs.size());
+                    columns.addAll(allColumns);
                     pkColumns = newLinkedHashSet(parent.getPKColumns());
                 }
             } else {

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java
index 81f2119..5fc9d87 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PDataType.java
@@ -1544,6 +1544,15 @@ public enum PDataType {
             // No association of runtime byte size from decimal precision.
             return null;
         }
+        
+        @Override
+        public String toStringLiteral(byte[] b, int offset, int length, Format formatter) {
+            if (formatter == null) {
+                BigDecimal o = (BigDecimal)toObject(b, offset, length);
+                return o.toPlainString();
+            }
+            return super.toStringLiteral(b,offset, length, formatter);
+        }
     },
     TIMESTAMP("TIMESTAMP", Types.TIMESTAMP, Timestamp.class, new DateCodec()) {
 

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/test/java/org/apache/phoenix/end2end/BaseViewTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/BaseViewTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/BaseViewTest.java
new file mode 100644
index 0000000..d9d9255
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/BaseViewTest.java
@@ -0,0 +1,133 @@
+/*
+ * 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.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.math.BigDecimal;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.util.Map;
+
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.util.QueryUtil;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.junit.BeforeClass;
+
+import com.google.common.collect.Maps;
+
+public class BaseViewTest extends BaseHBaseManagedTimeTest {
+
+    @BeforeClass 
+    public static void doSetup() throws Exception {
+        Map<String,String> props = Maps.newHashMapWithExpectedSize(1);
+        // Don't split intra region so we can more easily know that the n-way parallelization is for the explain plan
+        props.put(QueryServices.MAX_INTRA_REGION_PARALLELIZATION_ATTRIB, Integer.toString(1));
+        // Must update config before starting server
+        startServer(getUrl(), new ReadOnlyProps(props.entrySet().iterator()));
+    }
+
+    protected void testUpdatableViewWithIndex(Integer saltBuckets) throws Exception {
+        testUpdatableView(saltBuckets);
+        testUpdatableViewIndex(saltBuckets);
+    }
+
+    protected void testUpdatableView(Integer saltBuckets) throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        String ddl = "CREATE TABLE t (k1 INTEGER NOT NULL, k2 INTEGER NOT NULL, k3 DECIMAL, s VARCHAR CONSTRAINT pk PRIMARY KEY (k1, k2, k3))" + (saltBuckets == null ? "" : (" SALT_BUCKETS="+saltBuckets));
+        conn.createStatement().execute(ddl);
+        ddl = "CREATE VIEW v AS SELECT * FROM t WHERE k1 = 1";
+        conn.createStatement().execute(ddl);
+        for (int i = 0; i < 10; i++) {
+            conn.createStatement().execute("UPSERT INTO t VALUES(" + (i % 4) + "," + (i+100) + "," + (i > 5 ? 2 : 1) + ")");
+        }
+        conn.commit();
+        ResultSet rs;
+        
+        rs = conn.createStatement().executeQuery("SELECT count(*) FROM t");
+        assertTrue(rs.next());
+        assertEquals(10, rs.getInt(1));
+        rs = conn.createStatement().executeQuery("SELECT count(*) FROM v");
+        assertTrue(rs.next());
+        assertEquals(3, rs.getInt(1));
+        rs = conn.createStatement().executeQuery("SELECT k1, k2, k3 FROM v");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(101, rs.getInt(2));
+        assertEquals(1, rs.getInt(3));
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(105, rs.getInt(2));
+        assertEquals(1, rs.getInt(3));
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(109, rs.getInt(2));
+        assertEquals(2, rs.getInt(3));
+        assertFalse(rs.next());
+
+        conn.createStatement().execute("UPSERT INTO v(k2,S,k3) VALUES(120,'foo',50.0)");
+        conn.createStatement().execute("UPSERT INTO v(k2,S,k3) VALUES(121,'bar',51.0)");
+        conn.commit();
+        rs = conn.createStatement().executeQuery("SELECT k1, k2 FROM v WHERE k2 >= 120");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(120, rs.getInt(2));
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(121, rs.getInt(2));
+        assertFalse(rs.next());
+    }
+
+    protected void testUpdatableViewIndex(Integer saltBuckets) throws Exception {
+        ResultSet rs;
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute("CREATE INDEX i1 on v(k3) include (s)");
+        String query = "SELECT k1, k2, k3, s FROM v WHERE k3 = 51.0";
+        rs = conn.createStatement().executeQuery(query);
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(121, rs.getInt(2));
+        assertTrue(BigDecimal.valueOf(51.0).compareTo(rs.getBigDecimal(3))==0);
+        assertEquals("bar", rs.getString(4));
+        assertFalse(rs.next());
+        rs = conn.createStatement().executeQuery("EXPLAIN " + query);
+        assertEquals(saltBuckets == null
+                ? "CLIENT PARALLEL 1-WAY RANGE SCAN OVER I1 [" + Short.MIN_VALUE + ",51]"
+                : "CLIENT PARALLEL " + saltBuckets + "-WAY SKIP SCAN ON 3 KEYS OVER I1 [0," + Short.MIN_VALUE + ",51] - [2," + Short.MIN_VALUE + ",51]\nCLIENT MERGE SORT",
+            QueryUtil.getExplainPlan(rs));
+
+        conn.createStatement().execute("CREATE INDEX i2 on v(s)");
+        query = "SELECT k1, k2, s FROM v WHERE s = 'foo'";
+        rs = conn.createStatement().executeQuery(query);
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(120, rs.getInt(2));
+        assertEquals("foo", rs.getString(3));
+        assertFalse(rs.next());
+        rs = conn.createStatement().executeQuery("EXPLAIN " + query);
+        assertEquals(saltBuckets == null
+                ? "CLIENT PARALLEL 1-WAY RANGE SCAN OVER I2 [" + (Short.MIN_VALUE+1) + ",'foo']"
+                : "CLIENT PARALLEL " + saltBuckets + "-WAY SKIP SCAN ON 3 KEYS OVER I2 [0," + (Short.MIN_VALUE+1) + ",'foo'] - [2," + (Short.MIN_VALUE+1) + ",'foo']\nCLIENT MERGE SORT",
+            QueryUtil.getExplainPlan(rs));
+    }
+
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/test/java/org/apache/phoenix/end2end/SaltedViewTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/SaltedViewTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/SaltedViewTest.java
new file mode 100644
index 0000000..41eff4f
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/SaltedViewTest.java
@@ -0,0 +1,35 @@
+/*
+ * 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 org.junit.Test;
+
+public class SaltedViewTest extends BaseViewTest {
+    
+    /**
+     * Salted tests must be in their own test file to ensure that the underlying
+     * table is dropped. Otherwise, the splits may not be performed.
+     * TODO: we should throw in that case
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testSaltedUpdatableViewWithIndex() throws Exception {
+        testUpdatableViewWithIndex(3);
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/c439d3a0/phoenix-core/src/test/java/org/apache/phoenix/end2end/ViewTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ViewTest.java b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ViewTest.java
index da21389..faded93 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/end2end/ViewTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/end2end/ViewTest.java
@@ -31,8 +31,8 @@ import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.schema.ReadOnlyTableException;
 import org.junit.Test;
 
-public class ViewTest extends BaseHBaseManagedTimeTest {
-
+public class ViewTest extends BaseViewTest {
+    
     @Test
     public void testReadOnlyView() throws Exception {
         Connection conn = DriverManager.getConnection(getUrl());
@@ -84,48 +84,13 @@ public class ViewTest extends BaseHBaseManagedTimeTest {
     }
 
     @Test
-    public void testUpdatableView() throws Exception {
-        Connection conn = DriverManager.getConnection(getUrl());
-        String ddl = "CREATE TABLE t (k1 INTEGER NOT NULL, k2 INTEGER NOT NULL, k3 DECIMAL, CONSTRAINT pk PRIMARY KEY (k1, k2, k3))";
-        conn.createStatement().execute(ddl);
-        ddl = "CREATE VIEW v AS SELECT * FROM t WHERE k1 = 1";
-        conn.createStatement().execute(ddl);
-        for (int i = 0; i < 10; i++) {
-            conn.createStatement().execute("UPSERT INTO t VALUES(" + (i % 4) + "," + (i+100) + "," + (i > 5 ? 2 : 1) + ")");
-        }
-        conn.commit();
-        
-        ResultSet rs = conn.createStatement().executeQuery("SELECT k1, k2, k3 FROM v");
-        assertTrue(rs.next());
-        assertEquals(1, rs.getInt(1));
-        assertEquals(101, rs.getInt(2));
-        assertEquals(1, rs.getInt(3));
-        assertTrue(rs.next());
-        assertEquals(1, rs.getInt(1));
-        assertEquals(105, rs.getInt(2));
-        assertEquals(1, rs.getInt(3));
-        assertTrue(rs.next());
-        assertEquals(1, rs.getInt(1));
-        assertEquals(109, rs.getInt(2));
-        assertEquals(2, rs.getInt(3));
-        assertFalse(rs.next());
-
-        conn.createStatement().execute("UPSERT INTO v(k2) VALUES(120)");
-        conn.createStatement().execute("UPSERT INTO v(k2) VALUES(121)");
-        conn.commit();
-        rs = conn.createStatement().executeQuery("SELECT k1, k2 FROM v WHERE k2 >= 120");
-        assertTrue(rs.next());
-        assertEquals(1, rs.getInt(1));
-        assertEquals(120, rs.getInt(2));
-        assertTrue(rs.next());
-        assertEquals(1, rs.getInt(1));
-        assertEquals(121, rs.getInt(2));
-        assertFalse(rs.next());
+    public void testNonSaltedUpdatableViewWithIndex() throws Exception {
+        testUpdatableViewWithIndex(null);
     }
-
+    
     @Test
     public void testUpdatableOnUpdatableView() throws Exception {
-        testUpdatableView();
+        testUpdatableView(null);
         Connection conn = DriverManager.getConnection(getUrl());
         String ddl = "CREATE VIEW v2 AS SELECT * FROM v WHERE k3 = 2";
         conn.createStatement().execute(ddl);
@@ -148,9 +113,9 @@ public class ViewTest extends BaseHBaseManagedTimeTest {
 
     @Test
     public void testReadOnlyOnUpdatableView() throws Exception {
-        testUpdatableView();
+        testUpdatableView(null);
         Connection conn = DriverManager.getConnection(getUrl());
-        String ddl = "CREATE VIEW v2 AS SELECT * FROM v WHERE k3 > 1";
+        String ddl = "CREATE VIEW v2 AS SELECT * FROM v WHERE k3 > 1 and k3 < 50";
         conn.createStatement().execute(ddl);
         ResultSet rs = conn.createStatement().executeQuery("SELECT k1, k2, k3 FROM v2");
         assertTrue(rs.next());