You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/11/17 23:59:21 UTC
[GitHub] [phoenix] yanxinyi opened a new pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
yanxinyi opened a new pull request #972:
URL: https://github.com/apache/phoenix/pull/972
…ndex on view
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530084220
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver extends BaseRegionObserver {
Review comment:
maybe in the future, we want to do something specific for the syacat table.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530085070
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver 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);
+ if (clientVersion != UNKNOWN_CLIENT_VERSION) {
Review comment:
since corpoc is not only for the phoenix, but it also works for the HBase. We don't want to change the behavior other than the Phoenix environment.
The ScanUtil.getClientVersion returns UNKNOWN_CLIENT_VERSION if the phoenix client version is not setup.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530710911
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SystemCatalogViewIndexIdFilter.java
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SystemCatalogViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SystemCatalogViewIndexIdFilter() {
+ }
+
+ public SystemCatalogViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ /*
+ We retrieve the VIEW_INDEX_ID cell from SMALLINT to BIGINT or BIGINT to SMALLINT if and
+ only if VIEW_INDEX_ID is included as part of the projected column.
+
+ This is combination of diff client created view index looks like:
+
+ client VIEW_INDEX_ID(Cell number of bytes) VIEW_INDEX_ID_DATA_TYPE
+ pre-4.14 2 bytes NULL
Review comment:
right. fixed typo
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-733394782
Had a quick glance and added some review comments. As discussed offline, please add more comments based on various `if/else` conditions for when we do the conversion vs. when we don't/can't. Will review in more detail later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530083783
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
+ private String ddlForBaseTable = "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 String ddlForView = "CREATE VIEW %s (A BIGINT PRIMARY KEY, B BIGINT)" +
+ " AS SELECT * FROM %s WHERE ID='ABC'";
+ private String ddlForViewIndex =
+ "CREATE INDEX %s ON %s (B DESC) INCLUDE (NUM)";
+ private String selectAll = "SELECT * FROM SYSTEM.CATALOG";
+ private String selectRow = "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(ddlForBaseTable, fullTableName));
+ stmt.execute(String.format(ddlForView, viewFullName, fullTableName));
+ stmt.execute(String.format(ddlForViewIndex, viewIndexName, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, viewIndexName));
+ rs.next();
+ 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(ddlForBaseTable, fullTableName));
+ stmt.execute(String.format(ddlForView, viewFullName, fullTableName));
+ stmt.execute(String.format(ddlForViewIndex, viewIndexName1, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, 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(ddlForViewIndex, viewIndexName2, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, 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(selectAll));
Review comment:
because we want to test the mixed cases.
in this test, we create two views (one with smallint config, and one with bigint config)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r528968696
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SyscatViewIndexIdFilter() {
+ }
+
+ public SyscatViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ // pre-splittable client should always using SMALLINT
Review comment:
nit: Change to "pre-4.15 client" instead
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SyscatViewIndexIdFilter() {
+ }
+
+ public SyscatViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ // pre-splittable client should always using SMALLINT
+ if (type == NULL_DATA_TYPE_VALUE && viewIndexIdCell.getValueLength() >
+ VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN) {
+ Cell keyValue = ViewIndexIdRetrieveUtil.
Review comment:
What if there is a legitimate long number (not just extra bits, but actually large) here. We will shorten it to a SMALLINT and it becomes unusable right? Although based on offline discussions, seems like that case already means this view index ID is unqueryable. Should we trace log this or track it via a metric or something?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SyscatViewIndexIdFilter() {
+ }
+
+ public SyscatViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ // pre-splittable client should always using SMALLINT
+ if (type == NULL_DATA_TYPE_VALUE && viewIndexIdCell.getValueLength() >
+ VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN) {
+ Cell keyValue = ViewIndexIdRetrieveUtil.
+ getViewIndexIdKeyValueInShortDataFormat(viewIndexIdCell);
+ Collections.replaceAll(kvs, viewIndexIdCell, keyValue);
+ }
+ } else {
+ // post-splittable client should always using BIGINT
Review comment:
nit: Change to "4.15+ client" instead
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni edited a comment on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni edited a comment on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-733394782
Had a quick glance and added some review comments. As discussed offline, please add more comments based on various `if/else` conditions for when we do the conversion vs. when we don't/can't.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-734080383
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 11s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 11m 0s | 4.x passed |
| +1 :green_heart: | compile | 0m 55s | 4.x passed |
| +1 :green_heart: | checkstyle | 1m 24s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 44s | 4.x passed |
| +0 :ok: | spotbugs | 3m 17s | phoenix-core in 4.x has 950 extant spotbugs warnings. |
| -0 :warning: | patch | 3m 24s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 34s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -1 :x: | checkstyle | 1m 38s | phoenix-core: The patch generated 83 new + 2517 unchanged - 1 fixed = 2600 total (was 2518) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 49s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 11s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 214m 53s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 35s | The patch does not generate ASF License warnings. |
| | | 248m 53s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.InQueryIT |
| | phoenix.end2end.IndexExtendedIT |
| | phoenix.end2end.AlterTableWithViewsIT |
| | phoenix.end2end.AggregateQueryIT |
| | phoenix.end2end.index.IndexMetadataIT |
| | phoenix.end2end.ConcurrentMutationsExtendedIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/972 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 14cd02cf6827 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / 7ac4dff |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/5/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/5/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/5/testReport/ |
| Max. process+thread count | 6230 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/5/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi closed pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi closed pull request #972:
URL: https://github.com/apache/phoenix/pull/972
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] jpisaac commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
jpisaac commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r527836747
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
##########
@@ -763,6 +770,18 @@ protected QueryPlan compileSingleFlatQuery(
if(plan instanceof BaseQueryPlan){
((BaseQueryPlan) plan).setApplicable(isApplicable);
}
+ if (context.getScan().getFilter() != null &&
Review comment:
Maybe we can push this filter in preScannerOpen of the RegionScanner in Coproc, then it will be an all server-side fix
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi edited a comment on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi edited a comment on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-731417547
Added filter at the Coproc preScannerOpen method as we discussed offline. Please take a look again, thanks!
cc @jpisaac @ChinmaySKulkarni @kadirozde @lhofhansl
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530697105
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.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 SyscatRegionObserver 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
+ didn't set. We only want to retrieve the data based on the client version, and we don't
Review comment:
nit: *isn't
##########
File path: 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 BaseOwnClusterIT {
Review comment:
The comment in `BaseOwnClusterIT` says "Any new integration tests that need their own mini cluster should be extending {@link BaseUniqueNamesOwnClusterIT} class directly" so maybe we want to do that and then use uniqueNames for all table/view/index names etc.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SystemCatalogViewIndexIdFilter.java
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SystemCatalogViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SystemCatalogViewIndexIdFilter() {
+ }
+
+ public SystemCatalogViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ /*
+ We retrieve the VIEW_INDEX_ID cell from SMALLINT to BIGINT or BIGINT to SMALLINT if and
+ only if VIEW_INDEX_ID is included as part of the projected column.
+
+ This is combination of diff client created view index looks like:
+
+ client VIEW_INDEX_ID(Cell number of bytes) VIEW_INDEX_ID_DATA_TYPE
+ pre-4.14 2 bytes NULL
+ post-4.15[config smallint] 2 bytes 5(smallint)
+ post-4.15[config bigint] 8 bytes -5(bigint)
+ */
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ /*
+ For pre-4.15 client select query cannot include VIEW_INDEX_ID_DATA_TYPE as part
+ of the projected columns; for this reason, the TYPE will always be NULL. Since
+ the pre-4.15 client always assume the VIEW_INDEX_ID column is type of SMALLINT,
+ we need to retrieve the BIGINT cell to SMALLINT cell.
+
+ VIEW_INDEX_ID_DATA_TYPE, VIEW_INDEX_ID(Cell representation of the data)
+ NULL, SMALLINT -> DO NOT RETRIEVE
Review comment:
By DO NOT RETRIEVE, you mean DO NOT CONVERT right? Can you please clarify this in comments?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SystemCatalogViewIndexIdFilter.java
##########
@@ -0,0 +1,160 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SystemCatalogViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SystemCatalogViewIndexIdFilter() {
+ }
+
+ public SystemCatalogViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ /*
+ We retrieve the VIEW_INDEX_ID cell from SMALLINT to BIGINT or BIGINT to SMALLINT if and
+ only if VIEW_INDEX_ID is included as part of the projected column.
+
+ This is combination of diff client created view index looks like:
+
+ client VIEW_INDEX_ID(Cell number of bytes) VIEW_INDEX_ID_DATA_TYPE
+ pre-4.14 2 bytes NULL
Review comment:
this should be pre-4.15 right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530710641
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver extends BaseRegionObserver {
Review comment:
sure. renamed to `SystemCatalogRegionObserver`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530041005
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
+ private String ddlForBaseTable = "CREATE TABLE %s (TENANT_ID CHAR(15) NOT NULL, ID CHAR(3)" +
Review comment:
nit: Can we make all of these static final Strings?
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
+ private String ddlForBaseTable = "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 String ddlForView = "CREATE VIEW %s (A BIGINT PRIMARY KEY, B BIGINT)" +
+ " AS SELECT * FROM %s WHERE ID='ABC'";
+ private String ddlForViewIndex =
+ "CREATE INDEX %s ON %s (B DESC) INCLUDE (NUM)";
+ private String selectAll = "SELECT * FROM SYSTEM.CATALOG";
+ private String selectRow = "SELECT VIEW_INDEX_ID,VIEW_INDEX_ID_DATA_TYPE" +
Review comment:
nit: Since these Strings are constants, by convention we can snake case capitalize them.
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
Review comment:
Can you add a class-level comment and mention the JIRA(s) involved in this for more context
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
+ private String ddlForBaseTable = "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 String ddlForView = "CREATE VIEW %s (A BIGINT PRIMARY KEY, B BIGINT)" +
+ " AS SELECT * FROM %s WHERE ID='ABC'";
+ private String ddlForViewIndex =
+ "CREATE INDEX %s ON %s (B DESC) INCLUDE (NUM)";
+ private String selectAll = "SELECT * FROM SYSTEM.CATALOG";
+ private String selectRow = "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(ddlForBaseTable, fullTableName));
+ stmt.execute(String.format(ddlForView, viewFullName, fullTableName));
+ stmt.execute(String.format(ddlForViewIndex, viewIndexName, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, viewIndexName));
+ rs.next();
+ assertEquals(Short.MIN_VALUE,rs.getLong(1));
Review comment:
Even when the `LONG_VIEW_INDEX_ENABLED_ATTRIB ` config is set to long, we expect Short.MIN_VALUE to allow clients to switch the config (as we discussed offline). Can you add a comment about this in the test?
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
+ private String ddlForBaseTable = "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 String ddlForView = "CREATE VIEW %s (A BIGINT PRIMARY KEY, B BIGINT)" +
+ " AS SELECT * FROM %s WHERE ID='ABC'";
+ private String ddlForViewIndex =
+ "CREATE INDEX %s ON %s (B DESC) INCLUDE (NUM)";
+ private String selectAll = "SELECT * FROM SYSTEM.CATALOG";
+ private String selectRow = "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(ddlForBaseTable, fullTableName));
+ stmt.execute(String.format(ddlForView, viewFullName, fullTableName));
+ stmt.execute(String.format(ddlForViewIndex, viewIndexName, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, viewIndexName));
+ rs.next();
+ 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(ddlForBaseTable, fullTableName));
+ stmt.execute(String.format(ddlForView, viewFullName, fullTableName));
+ stmt.execute(String.format(ddlForViewIndex, viewIndexName1, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, 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(ddlForViewIndex, viewIndexName2, viewFullName));
+
+ ResultSet rs = stmt.executeQuery(String.format(selectRow, 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(selectAll));
Review comment:
Why not just query for the exact metadata rows for the view indices instead of select * .
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
##########
@@ -17,8 +17,14 @@
*/
package org.apache.phoenix.compile;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID;
import static org.apache.phoenix.query.QueryServices.WILDCARD_QUERY_DYNAMIC_COLS_ATTRIB;
import static org.apache.phoenix.query.QueryServicesOptions.DEFAULT_WILDCARD_QUERY_DYNAMIC_COLS_ATTRIB;
+import static org.apache.phoenix.schema.types.PDataType.TRUE_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.SYSCATA_COPROC_IGNORE_TAG;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.SYSCATA_COPROC_IGNORE_BYTE;
Review comment:
These imports look unused. Please remove
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIndexIdRetrieveIT.java
##########
@@ -0,0 +1,169 @@
+/*
+ * 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;
+
+public class ViewIndexIdRetrieveIT extends ParallelStatsDisabledIT {
Review comment:
Since you need to query `SYSTEM.CATALOG` we would need to make this test use its own mini-cluster to prevent flapping in case of other parallel test runs
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver extends BaseRegionObserver {
Review comment:
Can we rename this coproc so it indicates that it is for view_index_id queries on system.catalog instead of this generic name?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SyscatViewIndexIdFilter() {
+ }
+
+ public SyscatViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ // pre-splittable client should always using SMALLINT
+ if (type == NULL_DATA_TYPE_VALUE && viewIndexIdCell.getValueLength() >
+ VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN) {
+ Cell keyValue = ViewIndexIdRetrieveUtil.
+ getViewIndexIdKeyValueInShortDataFormat(viewIndexIdCell);
+ Collections.replaceAll(kvs, viewIndexIdCell, keyValue);
+ }
+ } else {
+ // post-splittable client should always using BIGINT
+ if (type != Types.BIGINT && viewIndexIdCell.getValueLength() <
Review comment:
Let's add comments for these various conditions
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver 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);
+ if (clientVersion != UNKNOWN_CLIENT_VERSION) {
Review comment:
nit: Not sure I understand why we don't care about 4.4.0 clients (not that it matters really). What is it for?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
Review comment:
nit: Rename to "SystemCatalog..." instead of "SysCat.."
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r527964134
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
##########
@@ -763,6 +770,18 @@ protected QueryPlan compileSingleFlatQuery(
if(plan instanceof BaseQueryPlan){
((BaseQueryPlan) plan).setApplicable(isApplicable);
}
+ if (context.getScan().getFilter() != null &&
Review comment:
added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-733557546
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 43s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 12m 22s | 4.x passed |
| +1 :green_heart: | compile | 1m 1s | 4.x passed |
| +1 :green_heart: | checkstyle | 1m 7s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 49s | 4.x passed |
| +0 :ok: | spotbugs | 3m 39s | phoenix-core in 4.x has 950 extant spotbugs warnings. |
| -0 :warning: | patch | 3m 47s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| -1 :x: | checkstyle | 1m 11s | phoenix-core: The patch generated 82 new + 2512 unchanged - 1 fixed = 2594 total (was 2513) |
| +1 :green_heart: | whitespace | 0m 1s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 33s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 216m 14s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 49s | The patch does not generate ASF License warnings. |
| | | 254m 13s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.IndexToolIT |
| | phoenix.end2end.index.LocalIndexIT |
| | phoenix.end2end.IndexToolForNonTxGlobalIndexIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/972 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 6a158be4faaf 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / c3818ee |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/4/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/4/testReport/ |
| Max. process+thread count | 6341 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/4/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530100936
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/filter/SyscatViewIndexIdFilter.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.filter;
+
+import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.exceptions.DeserializationException;
+import org.apache.hadoop.hbase.filter.FilterBase;
+import org.apache.hadoop.hbase.util.Writables;
+import org.apache.hadoop.io.Writable;
+import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
+import org.apache.phoenix.schema.SortOrder;
+import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.util.KeyValueUtil;
+import org.apache.phoenix.util.ViewIndexIdRetrieveUtil;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.Types;
+import java.util.Collections;
+import java.util.List;
+
+
+import static org.apache.phoenix.coprocessor.MetaDataProtocol.MIN_SPLITTABLE_SYSTEM_CATALOG;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_DATA_TYPE_BYTES;
+import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.NULL_DATA_TYPE_VALUE;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_BIGINT_TYPE_PTR_LEN;
+import static org.apache.phoenix.util.ViewIndexIdRetrieveUtil.VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN;
+
+public class SyscatViewIndexIdFilter extends FilterBase implements Writable {
+ private int clientVersion;
+
+ public SyscatViewIndexIdFilter() {
+ }
+
+ public SyscatViewIndexIdFilter(int clientVersion) {
+ this.clientVersion = clientVersion;
+ }
+
+ @Override
+ public ReturnCode filterKeyValue(Cell keyValue) {
+ return ReturnCode.INCLUDE_AND_NEXT_COL;
+ }
+
+ @Override
+ public boolean hasFilterRow() {
+ return true;
+ }
+
+ @Override
+ public void filterRowCells(List<Cell> kvs) throws IOException {
+ Cell viewIndexIdCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_BYTES);
+
+ if (viewIndexIdCell != null) {
+ int type = NULL_DATA_TYPE_VALUE;
+ Cell viewIndexIdDataTypeCell = KeyValueUtil.getColumnLatest(
+ GenericKeyValueBuilder.INSTANCE, kvs,
+ DEFAULT_COLUMN_FAMILY_BYTES, VIEW_INDEX_ID_DATA_TYPE_BYTES);
+ if (viewIndexIdDataTypeCell != null) {
+ type = (Integer) PInteger.INSTANCE.toObject(
+ viewIndexIdDataTypeCell.getValueArray(),
+ viewIndexIdDataTypeCell.getValueOffset(),
+ viewIndexIdDataTypeCell.getValueLength(),
+ PInteger.INSTANCE,
+ SortOrder.ASC);
+ }
+ if (this.clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ // pre-splittable client should always using SMALLINT
+ if (type == NULL_DATA_TYPE_VALUE && viewIndexIdCell.getValueLength() >
+ VIEW_INDEX_ID_SMALLINT_TYPE_VALUE_LEN) {
+ Cell keyValue = ViewIndexIdRetrieveUtil.
Review comment:
Can you provide an example of a legitimate long number? Do you mean when a value here is greater than the SHORT.MAX_VALUE? Yeah, if that's the case the value that we send back to the client will lose some data precision but still queryable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530084220
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver extends BaseRegionObserver {
Review comment:
if we want to do something special for the syscat in the future, we don't need to create a new coproc. In fact, we can add on new logic to this existing coproc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r533759719
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SystemCatalogRegionObserver.java
##########
@@ -0,0 +1,59 @@
+/*
Review comment:
We do not have a new coproc for updating the scan filter. We can add a boolean scan attribute for adding SystemCatalogViewIndexIdFilter and then update the scan filter in BaseScannerRegionObserver#preScannerOpen(). After updating the scan filter, you can set the attribute to the false to make sure that we update the scan filter once.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530654596
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SyscatRegionObserver.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.SyscatViewIndexIdFilter;
+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 SyscatRegionObserver extends BaseRegionObserver {
Review comment:
Ok makes sense. Can you rename it to `SystemCatalogRegionObserver` in that case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-731417547
Added filter at the Coproc preScannerOpen method as we discussed offline. Please take a look again, thanks!
cc @jpisaac @ChinmaySKulkarni @kadirozde
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-731480592
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 4s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 12m 13s | 4.x passed |
| +1 :green_heart: | compile | 1m 0s | 4.x passed |
| +1 :green_heart: | checkstyle | 1m 13s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 45s | 4.x passed |
| +0 :ok: | spotbugs | 3m 11s | phoenix-core in 4.x has 950 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 19s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| -1 :x: | checkstyle | 1m 13s | phoenix-core: The patch generated 80 new + 2509 unchanged - 3 fixed = 2589 total (was 2512) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 44s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 23s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 181m 47s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 29s | The patch does not generate ASF License warnings. |
| | | 216m 52s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.tx.TxCheckpointIT |
| | phoenix.end2end.index.SaltedIndexIT |
| | phoenix.end2end.AlterTableWithViewsIT |
| | phoenix.end2end.IndexScrutinyToolForTenantIT |
| | phoenix.end2end.PhoenixRuntimeIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/972 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 7a72de256505 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / e57fcc8 |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/3/testReport/ |
| Max. process+thread count | 5981 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/3/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r530710145
##########
File path: 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 BaseOwnClusterIT {
Review comment:
changed to `BaseUniqueNamesOwnClusterIT`.
All table/view/index are all using generateUniqueName method to get a unique name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] kadirozde commented on a change in pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
kadirozde commented on a change in pull request #972:
URL: https://github.com/apache/phoenix/pull/972#discussion_r534661019
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/SystemCatalogRegionObserver.java
##########
@@ -0,0 +1,59 @@
+/*
Review comment:
After a side conversation with @yanxinyi, I realized that we need a new coproc in order to identify a syscat table region as we need to support old clients and we cannot expect on old client to set a new scan attributes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #972: PHOENIX-5712 Got SYSCAT ILLEGAL_DATA exception after created tenant i…
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #972:
URL: https://github.com/apache/phoenix/pull/972#issuecomment-729659093
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 18s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 11m 44s | 4.x passed |
| +1 :green_heart: | compile | 1m 3s | 4.x passed |
| +1 :green_heart: | checkstyle | 1m 13s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 48s | 4.x passed |
| +0 :ok: | spotbugs | 3m 15s | phoenix-core in 4.x has 946 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 3s | the patch passed |
| +1 :green_heart: | javac | 1m 3s | the patch passed |
| -1 :x: | checkstyle | 1m 16s | phoenix-core: The patch generated 141 new + 2516 unchanged - 32 fixed = 2657 total (was 2548) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 31s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 200m 27s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. |
| | | 236m 1s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.ViewIndexIdRetrieveIT |
| | phoenix.end2end.NullIT |
| | phoenix.end2end.DropIndexedColsIT |
| | phoenix.end2end.BackwardCompatibilityIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/972 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux da6c1ab7e83a 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / 510ca96 |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/2/testReport/ |
| Max. process+thread count | 6074 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-972/2/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org