PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…

phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java

@@ -22,11 +22,13 @@
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.CREATE_ADD;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.CREATE_DIVERGED_VIEW;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.INDEX_REBUILD_ASYNC;
+import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.ADD_VIEW_INDEX;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_ADD_DATA;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_ADD_DELETE;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_CREATE_ADD;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_CREATE_DIVERGED_VIEW;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_INDEX_REBUILD_ASYNC;
+import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.QUERY_VIEW_INDEX;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.assertExpectedOutput;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.checkForPreConditions;
 import static org.apache.phoenix.end2end.BackwardCompatibilityTestUtil.computeClientVersions;
@@ -50,6 +52,7 @@
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Admin;
 import org.apache.phoenix.coprocessor.TaskMetaDataEndpoint;
+import org.apache.phoenix.coprocessor.SystemCatalogRegionObserver;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
 import org.apache.phoenix.jdbc.PhoenixDriver;
 import org.apache.phoenix.query.QueryServices;
@@ -374,4 +377,34 @@ public void testSystemTaskCreationWithIndexAsyncRebuild() throws Exception {
         }
     }
 
+    @Test
+    public void testViewIndexIdCreatedWithOldClient() throws Exception {
+        executeQueryWithClientVersion(compatibleClientVersion, ADD_VIEW_INDEX, zkQuorum);
+        try (org.apache.hadoop.hbase.client.Connection conn =
+                     hbaseTestUtil.getConnection(); Admin admin = conn.getAdmin()) {
+            HTableDescriptor tableDescriptor = admin.getTableDescriptor(
+                    TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME));
+            assertFalse("Coprocessor " + SystemCatalogRegionObserver.class.getName()
+                    + " has been added with compatible client version: "
+                    + compatibleClientVersion, tableDescriptor.hasCoprocessor(
+                    SystemCatalogRegionObserver.class.getName()));
+
+            executeQueriesWithCurrentVersion(QUERY_VIEW_INDEX, url, NONE);
+            assertExpectedOutput(QUERY_VIEW_INDEX);
+
+            tableDescriptor = admin.getTableDescriptor(
+                    TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME));
+            assertTrue("Coprocessor " + SystemCatalogRegionObserver.class.getName()
+                    + " has been added with compatible client version: "
+                    + compatibleClientVersion, tableDescriptor.hasCoprocessor(
+                    SystemCatalogRegionObserver.class.getName()));
+        }
+    }
+
+    @Test
+    public void testViewIndexIdCreatedWithNewClient() throws Exception {
+        executeQueriesWithCurrentVersion(ADD_VIEW_INDEX, url, NONE);
+        executeQueryWithClientVersion(compatibleClientVersion, QUERY_VIEW_INDEX, zkQuorum);
+        assertExpectedOutput(QUERY_VIEW_INDEX);
+    }
 }

phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityTestUtil.java

@@ -74,14 +74,17 @@ public final class BackwardCompatibilityTestUtil {
     public static final String CREATE_ADD = "create_add";
     public static final String CREATE_TMP_TABLE = "create_tmp_table";
     public static final String CREATE_DIVERGED_VIEW = "create_diverged_view";
+    public static final String ADD_VIEW_INDEX = "add_view_index";
     public static final String ADD_DATA = "add_data";
     public static final String ADD_DELETE = "add_delete";
     public static final String DELETE = "delete";
+    public static final String VIEW_INDEX = "view_index";
     public static final String SELECT_AND_DROP_TABLE = "select_and_drop_table";
     public static final String QUERY_CREATE_ADD = QUERY_PREFIX + CREATE_ADD;
     public static final String QUERY_ADD_DATA = QUERY_PREFIX + ADD_DATA;
     public static final String QUERY_ADD_DELETE = QUERY_PREFIX + ADD_DELETE;
     public static final String QUERY_DELETE = QUERY_PREFIX + DELETE;
+    public static final String QUERY_VIEW_INDEX = QUERY_PREFIX + VIEW_INDEX;
     public static final String QUERY_SELECT_AND_DROP_TABLE = QUERY_PREFIX + SELECT_AND_DROP_TABLE;
     public static final String QUERY_CREATE_DIVERGED_VIEW = QUERY_PREFIX + CREATE_DIVERGED_VIEW;
     public static final String INDEX_REBUILD_ASYNC = "index_rebuild_async";

phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java

@@ -0,0 +1,177 @@
+/*
+ * 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.jdbc.PhoenixDatabaseMetaData.TABLE_NAME;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_SCHEM;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE;
+import static org.apache.phoenix.query.QueryServices.LONG_VIEW_INDEX_ENABLED_ATTRIB;
+import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.Statement;
+import java.sql.Types;
+import java.util.Map;
+import java.util.Properties;
+
+import com.google.common.collect.Maps;
+import org.apache.hadoop.hbase.regionserver.ScanInfoUtil;
+import org.apache.phoenix.util.PropertiesUtil;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/*
+    After 4.15 release, Phoenix introduced VIEW_INDEX_ID_COLUMN_TYPE and changed VIEW_INDEX_ID data
+    type from SMALLINT to BIGINT. However, SELECT from syscat doesn't have the right view index id
+    because the VIEW_INDEX_ID column always assume the data type is BIGINT. PHOENIX-5712 introduced
+    a coproc that checks the client request version and send it back to the client.
+    For more information, please see PHOENIX-3547, PHOENIX-5712
+ */
+public class ViewIndexIdRetrieveIT extends BaseUniqueNamesOwnClusterIT {
+    private final String BASE_TABLE_DDL = "CREATE TABLE %s (TENANT_ID CHAR(15) NOT NULL, " +
+            "ID CHAR(3) NOT NULL, NUM BIGINT CONSTRAINT PK PRIMARY KEY (TENANT_ID, ID))" +
+            " MULTI_TENANT = true, COLUMN_ENCODED_BYTES=0 ";
+    private final String VIEW_DDL = "CREATE VIEW %s (A BIGINT PRIMARY KEY, B BIGINT)" +
+            " AS SELECT * FROM %s WHERE ID='ABC'";
+    private final String VIEW_INDEX_DDL =
+            "CREATE INDEX %s ON %s (B DESC) INCLUDE (NUM)";
+    private final String SELECT_ALL = "SELECT * FROM SYSTEM.CATALOG";
+    private final String SELECT_ROW = "SELECT VIEW_INDEX_ID,VIEW_INDEX_ID_DATA_TYPE" +
+            " FROM SYSTEM.CATALOG WHERE TABLE_NAME='%s' AND COLUMN_COUNT IS NOT NULL";
+
+    @BeforeClass
+    public static void setUp() throws Exception {
+        Map<String, String> props = Maps.newHashMapWithExpectedSize(1);
+        props.put(ScanInfoUtil.PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY, Integer.toString(60*60));
+        setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
+    }
+
+    @Test
+    public void testSelectViewIndexIdAsLong() throws Exception {
+        testSelectViewIndexId(true);
+    }
+
+    @Test
+    public void testSelectViewIndexIdAsShort() throws Exception {
+        testSelectViewIndexId(false);
+    }
+
+    private void testSelectViewIndexId(boolean isTestingLongViewIndexId) throws Exception {
+        String val = isTestingLongViewIndexId ? "true" : "false";
+        int expectedDataType = isTestingLongViewIndexId ? Types.BIGINT : Types.SMALLINT;
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        props.setProperty(LONG_VIEW_INDEX_ENABLED_ATTRIB, val);
+        String schema = generateUniqueName();
+        String baseTable = generateUniqueName();
+        String fullTableName = SchemaUtil.getTableName(schema, baseTable);
+        String viewName = generateUniqueName();
+        String viewFullName = SchemaUtil.getTableName(schema, viewName);
+        String viewIndexName = generateUniqueName();
+        try (final Connection conn = DriverManager.getConnection(url,props);
+             final Statement stmt = conn.createStatement()) {
+            stmt.execute(String.format(BASE_TABLE_DDL, fullTableName));
+            stmt.execute(String.format(VIEW_DDL, viewFullName, fullTableName));
+            stmt.execute(String.format(VIEW_INDEX_DDL, viewIndexName, viewFullName));
+
+            ResultSet rs = stmt.executeQuery(String.format(SELECT_ROW, viewIndexName));
+            rs.next();
+            // even we enabled longViewIndex config, but the sequence always starts at smallest short
+            assertEquals(Short.MIN_VALUE,rs.getLong(1));
+            assertEquals(expectedDataType, rs.getInt(2));
+            assertFalse(rs.next());
+        }
+    }
+
+    @Test
+    public void testMixedCase() throws Exception {
+        // mixed case
+        Properties propsForLongType = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        propsForLongType.setProperty(LONG_VIEW_INDEX_ENABLED_ATTRIB, "true");
+        String schema = generateUniqueName();
+        String baseTable = generateUniqueName();
+        String fullTableName = SchemaUtil.getTableName(schema, baseTable);
+        String viewName = generateUniqueName();
+        String viewFullName = SchemaUtil.getTableName(schema, viewName);
+        String viewIndexName1 = generateUniqueName();
+
+        // view index id data type is long
+        try (final Connection conn = DriverManager.getConnection(url,propsForLongType);
+             final Statement stmt = conn.createStatement()) {
+            stmt.execute(String.format(BASE_TABLE_DDL, fullTableName));
+            stmt.execute(String.format(VIEW_DDL, viewFullName, fullTableName));
+            stmt.execute(String.format(VIEW_INDEX_DDL, viewIndexName1, viewFullName));
+
+            ResultSet rs = stmt.executeQuery(String.format(SELECT_ROW, viewIndexName1));
+            rs.next();
+            assertEquals(Short.MIN_VALUE,rs.getLong(1));
+            assertEquals(Types.BIGINT, rs.getInt(2));
+            assertFalse(rs.next());
+        }
+
+        // view index id data type is short
+        String viewIndexName2 = generateUniqueName();
+        Properties propsForShortType = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        propsForShortType.setProperty(LONG_VIEW_INDEX_ENABLED_ATTRIB, "false");
+        try (final Connection conn = DriverManager.getConnection(url,propsForShortType);
+             final Statement stmt = conn.createStatement()) {
+            stmt.execute(String.format(VIEW_INDEX_DDL, viewIndexName2, viewFullName));
+
+            ResultSet rs = stmt.executeQuery(String.format(SELECT_ROW, viewIndexName2));
+            rs.next();
+            assertEquals(Short.MIN_VALUE + 1,rs.getLong(1));
+            assertEquals(Types.SMALLINT, rs.getInt(2));
+            assertFalse(rs.next());
+        }
+
+        // check select * from syscat
+        try (final Connection conn = DriverManager.getConnection(url);
+             final Statement stmt = conn.createStatement()) {
+            ResultSet rs = stmt.executeQuery(String.format(SELECT_ALL));
+            boolean checkShort = false;
+            boolean checkLong = false;
+            while (rs.next()) {
+
+                String schemaName = rs.getString(TABLE_SCHEM);
+                long viewIndexId = rs.getLong(VIEW_INDEX_ID);
+                if (schemaName != null && schemaName.equals(schema) && viewIndexId != 0) {
+                    int viewIndexIdDataType = rs.getInt(VIEW_INDEX_ID_DATA_TYPE);
+                    String tableName = rs.getString(TABLE_NAME);
+                    if (tableName.equals(viewIndexName1)) {
+                        assertEquals(Short.MIN_VALUE, viewIndexId);
+                        assertEquals(Types.BIGINT, viewIndexIdDataType);
+                        checkLong = true;
+                    } else if (tableName.equals(viewIndexName2)) {
+                        assertEquals(Short.MIN_VALUE + 1, viewIndexId);
+                        assertEquals(Types.SMALLINT, viewIndexIdDataType);
+                        checkShort = true;
+                    }
+                }
+            }
+            assertTrue(checkLong);
+            assertTrue(checkShort);
+        }
+    }
+}

phoenix-core/src/it/resources/gold_files/gold_query_view_index.txt

@@ -0,0 +1,20 @@
+ /*
+ * 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.
+ */
+
+ 'VIEW_INDEX_ID'
+'-32768'

phoenix-core/src/it/resources/sql_files/add_view_index.sql

@@ -0,0 +1,21 @@
+/*
+ * 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.
+ */
+
+CREATE TABLE MY_SCHEMA.VIEW_INDEX_BASE_TABLE (TENANT_ID CHAR(15) NOT NULL, ID CHAR(3) NOT NULL, NUM BIGINT CONSTRAINT PK PRIMARY KEY (TENANT_ID, ID)) MULTI_TENANT = true;
+CREATE VIEW MY_SCHEMA.GLOBAL_VIEW (A BIGINT PRIMARY KEY, B BIGINT) AS SELECT * FROM MY_SCHEMA.VIEW_INDEX_BASE_TABLE WHERE ID='ABC';
+CREATE INDEX MY_SCHEMA_VIEW_INDEX ON MY_SCHEMA.GLOBAL_VIEW (B DESC) INCLUDE (NUM);

phoenix-core/src/it/resources/sql_files/query_view_index.sql

@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+
+SELECT VIEW_INDEX_ID FROM SYSTEM.CATALOG WHERE TABLE_NAME='MY_SCHEMA_VIEW_INDEX' AND TABLE_SCHEM='MY_SCHEMA' AND COLUMN_COUNT IS NOT NULL;

phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java

@@ -87,7 +87,6 @@
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
-import org.apache.phoenix.util.SchemaUtil;
 import com.google.common.base.Optional;
 
 

phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SystemCatalogRegionObserver.java

@@ -0,0 +1,59 @@
+/*
+ * 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.coprocessor;
+
+import org.apache.hadoop.hbase.CoprocessorEnvironment;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.coprocessor.BaseRegionObserver;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.regionserver.RegionScanner;
+import org.apache.phoenix.filter.SystemCatalogViewIndexIdFilter;
+import org.apache.phoenix.util.ScanUtil;
+
+import java.io.IOException;
+
+import static org.apache.phoenix.util.ScanUtil.UNKNOWN_CLIENT_VERSION;
+
+/**
+ * Coprocessor that checks whether the VIEW_INDEX_ID needs to retrieve.
+ */
+public class SystemCatalogRegionObserver extends BaseRegionObserver {
+    @Override public void start(CoprocessorEnvironment e) throws IOException {
+        super.start(e);
+    }
+
+    @Override public void stop(CoprocessorEnvironment e) throws IOException {
+        super.stop(e);
+    }
+
+    @Override
+    public RegionScanner preScannerOpen(ObserverContext<RegionCoprocessorEnvironment> e, Scan scan,
+                                        RegionScanner s) throws IOException {
+        int clientVersion = ScanUtil.getClientVersion(scan);
+        /*
+            ScanUtil.getClientVersion returns UNKNOWN_CLIENT_VERSION if the phoenix client version
+            isn't set. We only want to retrieve the data based on the client version, and we don't
+            want to change the behavior other than Phoenix env.
+         */
+        if (clientVersion != UNKNOWN_CLIENT_VERSION) {
+            ScanUtil.andFilterAtBeginning(scan, new SystemCatalogViewIndexIdFilter(clientVersion));
+        }
+        return s;
+    }
+}