[GitHub] phoenix pull request #232: Pull request for merging the encode columns branc...
Github user samarthjain closed the pull request at: https://github.com/apache/phoenix/pull/232 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #232: Pull request for merging the encode columns branc...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/232#discussion_r102769336 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/PositionBasedMultiKeyValueTuple.java --- @@ -0,0 +1,86 @@ +/* + * 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.schema.tuple; + +import static com.google.common.base.Preconditions.checkArgument; + +import java.util.List; + +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.io.ImmutableBytesWritable; + +/** + * Tuple that uses the --- End diff -- This is a new tuple implementation that is tied with the EncodedColumnQualifierCellsList. It essentially provides a way of getting hold of cell belonging to a cq/cf by doing a position based look up as opposed to a MultiKeyValueTuple where we have to do a binary search in the List. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix issue #232: Pull request for merging the encode columns branch to 4....
Github user samarthjain commented on the issue: https://github.com/apache/phoenix/pull/232 @jtaylor-sfdc @twdsilva please review --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #232: Pull request for merging the encode columns branc...
GitHub user samarthjain opened a pull request: https://github.com/apache/phoenix/pull/232 Pull request for merging the encode columns branch to 4.x-HBase-0.98 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/phoenix encodecolumns2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/phoenix/pull/232.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #232 commit b49fc0d1d684b7864ddfafa665ceadfcd53e424f Author: Samarth <samarth.j...@salesforce.com> Date: 2017-02-14T23:40:50Z PHOENIX-1598 Column encoding to save space and improve performance commit 4044378fabc836f48d1dc0ce045c0684272dcffc Author: Samarth <samarth.j...@salesforce.com> Date: 2017-02-15T02:43:10Z Fix test failures in partial index rebuild tool --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #:
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/commit/11c0c3b7b40f34f130554363a85ef0de7edd31a3#commitcomment-20281443 hey Thomas, can you tell me more about the test failure that this fixes. My local test run was successful. Did it fail after the rebase? If so, which test/s? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74842947 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexExpressionIT.java --- @@ -10,10 +10,7 @@ package org.apache.phoenix.end2end.index; import static org.apache.phoenix.query.QueryConstants.MILLIS_IN_DAY; -import static org.apache.phoenix.util.TestUtil.INDEX_DATA_SCHEMA; --- End diff -- Re-organize the imports here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74842182 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SubqueryUsingSortMergeJoinIT.java --- @@ -66,16 +66,16 @@ @RunWith(Parameterized.class) public class SubqueryUsingSortMergeJoinIT extends BaseHBaseManagedTimeIT { - + --- End diff -- Nothing seems to have changed in this file. Please revert it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74841351 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanAfterManualSplitIT.java --- @@ -43,7 +43,7 @@ import com.google.common.collect.Maps; -public class SkipScanAfterManualSplitIT extends BaseHBaseManagedTimeIT { +public class SkipScanAfterManualSplitIT extends BaseHBaseManagedTimeTableReuseIT { --- End diff -- I think table names can be randomized here. I would also get rid of tableName and tableNameBytes static constants. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix issue #189: PHOENIX-3036 Modify phoenix IT tests to extend BaseHBase...
Github user samarthjain commented on the issue: https://github.com/apache/phoenix/pull/189 This looks great now @prakul. Did you also make the changes for the test classes where there are member variables for storing table names? The best way to handle those would be to declare a method annotated with @Before which just sets the tableName = generateRandomString(). Are you getting a clean run when doing mvn clean install? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74808858 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java --- @@ -482,20 +482,22 @@ public void testBinaryNonnullableIndex() throws Exception { conn.close(); } } - + @Test public void testAsyncCreatedDate() throws Exception { +//Have to delete metaData tables because BaseHBaseManagedTimeTableReuseIT doesn't delete them after each test case , and tenant list will create issues between test cases --- End diff -- Remove the comment and the following line. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74678543 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/iterate/BaseResultIterators.java --- @@ -23,6 +23,7 @@ import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_FAILED_QUERY_COUNTER; import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_QUERY_TIMEOUT_COUNTER; import static org.apache.phoenix.util.ByteUtil.EMPTY_BYTE_ARRAY; +import static org.apache.phoenix.util.ByteUtil.getKeyRange; --- End diff -- Please revert changes to this file as they are only import related. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74652145 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/LocalIndexIT.java --- @@ -76,24 +76,18 @@ import com.google.common.collect.Maps; @RunWith(Parameterized.class) -public class LocalIndexIT extends BaseHBaseManagedTimeIT { - -private String schemaName="TEST"; +public class LocalIndexIT extends BaseHBaseManagedTimeTableReuseIT { private boolean isNamespaceMapped; -private String tableName = schemaName + ".T"; -private String indexTableName = schemaName + ".I"; -private String indexName = "I"; -private String indexPhysicalTableName; -private TableName physicalTableName; +String schemaName="TEST"; public LocalIndexIT(boolean isNamespaceMapped) { this.isNamespaceMapped = isNamespaceMapped; -this.physicalTableName = SchemaUtil.getPhysicalTableName(tableName.getBytes(), isNamespaceMapped); -this.indexPhysicalTableName = this.physicalTableName.getNameAsString(); +//this.physicalTableName = SchemaUtil.getPhysicalTableName(tableName.getBytes(), isNamespaceMapped); --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74651889 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java --- @@ -354,27 +355,29 @@ public void testAlterIndexWithLowerCaseName() throws Exception { Connection conn = DriverManager.getConnection(getUrl(), props); conn.setAutoCommit(false); String indexName = "\"lowerCaseIndex\""; +String indexDataTable = generateRandomString(); +//String indexName = generateRandomString(); --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74651475 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexExpressionIT.java --- @@ -515,22 +531,27 @@ public void testSelectDistinctMutableLocalIndex() throws Exception { } protected void helpTestSelectDistinct(boolean mutable, boolean localIndex) throws Exception { -String dataTableName = mutable ? MUTABLE_INDEX_DATA_TABLE : INDEX_DATA_TABLE; +String dataTableName = generateRandomString(); String fullDataTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + dataTableName; +String indexName = generateRandomString(); +String fullIndexTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + indexName; + + // String dataTableName = mutable ? MUTABLE_INDEX_DATA_TABLE : INDEX_DATA_TABLE; --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74651437 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexExpressionIT.java --- @@ -461,14 +472,19 @@ public void testGroupByCountMutableLocalIndex() throws Exception { } protected void helpTestGroupByCount(boolean mutable, boolean localIndex) throws Exception { -String dataTableName = mutable ? MUTABLE_INDEX_DATA_TABLE : INDEX_DATA_TABLE; +String dataTableName = generateRandomString(); String fullDataTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + dataTableName; +String indexName = generateRandomString(); +String fullIndexTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + indexName; + + // String dataTableName = mutable ? MUTABLE_INDEX_DATA_TABLE : INDEX_DATA_TABLE; --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74637772 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertBigValuesIT.java --- @@ -62,7 +62,7 @@ public void testIntegerPK() throws Exception { assertTrue(rs.next()); assertEquals(testNumbers.length, rs.getInt(1)); assertFalse(rs.next()); -select = "SELECT pk FROM PKIntValueTest where pk >= " + Integer.MIN_VALUE + +select = "SELECT pk FROM PKIntValueTest where pk >= " + Integer.MIN_VALUE + --- End diff -- Table name is not a random string. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74636561 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantIdTypeIT.java --- @@ -74,7 +74,8 @@ public TenantIdTypeIT(String dataType, String tenantId, String otherTenantId) { this.dataType = dataType; this.tenantId = tenantId; this.otherTenantId = otherTenantId; -String tbl = "foo" + dataType; +String tbl = generateRandomString(); +//String tbl = "foo" + dataType; --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74631717 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SortMergeJoinMoreIT.java --- @@ -39,10 +39,10 @@ import com.google.common.collect.Maps; -public class SortMergeJoinMoreIT extends BaseHBaseManagedTimeIT { +public class SortMergeJoinMoreIT extends BaseHBaseManagedTimeTableReuseIT { --- End diff -- It doesn't look like all test methods were converted to use generateRandomString() for table names. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74523807 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java --- @@ -246,15 +248,15 @@ public void testVarCharXIntInQuery() throws Exception { public void testPreSplitCompositeFixedKey() throws Exception { Connection conn = DriverManager.getConnection(getUrl()); try { -conn.createStatement().execute("create table test(key_1 char(3) not null, key_2 char(4) not null, v varchar(8) CONSTRAINT pk PRIMARY KEY (key_1,key_2)) split on('000','100','200')"); +conn.createStatement().execute("create table test2(key_1 char(3) not null, key_2 char(4) not null, v varchar(8) CONSTRAINT pk PRIMARY KEY (key_1,key_2)) split on('000','100','200')"); --- End diff -- Is there a reason why table name here wasn't generated randomly? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74523759 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java --- @@ -111,10 +113,10 @@ private void initSelectAfterUpsertTable(Connection conn) throws Exception { @Test public void testSkipScanFilterQuery() throws Exception { -String createTableDDL = "CREATE TABLE test" + "(col1 VARCHAR," + "col2 VARCHAR," + "col3 VARCHAR," +String createTableDDL = "CREATE TABLE test1" + "(col1 VARCHAR," + "col2 VARCHAR," + "col3 VARCHAR," --- End diff -- Is there a reason why table name here wasn't generated randomly? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74335513 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SpooledTmpFileDeleteIT.java --- @@ -38,11 +38,12 @@ -public class SpooledTmpFileDeleteIT extends BaseHBaseManagedTimeIT { +public class SpooledTmpFileDeleteIT extends BaseHBaseManagedTimeTableReuseIT { private Connection conn = null; private Properties props = null; private File spoolDir; + private String tableName; --- End diff -- No table names in member variables, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74335011 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorIT.java --- @@ -65,7 +65,7 @@ public void testRowValueConstructorInWhereWithEqualsExpression() throws Exception { long ts = nextTimestamp(); String tenantId = getOrganizationId(); -initATableValues(tenantId, getDefaultSplits(tenantId), null, ts); +initATableValues("aTable", tenantId, getDefaultSplits(tenantId), null, ts, getUrl()); --- End diff -- Static constant, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74313800 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java --- @@ -52,11 +52,11 @@ import com.google.common.collect.Maps; -public class ReverseScanIT extends BaseHBaseManagedTimeIT { -protected static final String ATABLE_INDEX_NAME = "ATABLE_IDX"; +public class ReverseScanIT extends BaseHBaseManagedTimeTableReuseIT { +protected static final String ATABLE_INDEX_NAME = generateRandomString(); @BeforeClass -@Shadower(classBeingShadowed = BaseHBaseManagedTimeIT.class) +@Shadower(classBeingShadowed = BaseHBaseManagedTimeTableReuseIT.class) --- End diff -- I think you can remove the doSetup() method altogether here. We don't need to disable and delete ATABLE_NAME any more. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74313609 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java --- @@ -52,11 +52,11 @@ import com.google.common.collect.Maps; -public class ReverseScanIT extends BaseHBaseManagedTimeIT { -protected static final String ATABLE_INDEX_NAME = "ATABLE_IDX"; +public class ReverseScanIT extends BaseHBaseManagedTimeTableReuseIT { +protected static final String ATABLE_INDEX_NAME = generateRandomString(); --- End diff -- The only test case that uses the index is testReverseScanIndex. Please move this there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312995 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/RegexpSubstrFunctionIT.java --- @@ -33,13 +33,15 @@ import org.junit.Test; -public class RegexpSubstrFunctionIT extends BaseHBaseManagedTimeIT { +public class RegexpSubstrFunctionIT extends BaseHBaseManagedTimeTableReuseIT { private int id; +String tableName; --- End diff -- No member variables for table names, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312941 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/RegexpReplaceFunctionIT.java --- @@ -33,13 +33,15 @@ import org.junit.Test; -public class RegexpReplaceFunctionIT extends BaseHBaseManagedTimeIT { +public class RegexpReplaceFunctionIT extends BaseHBaseManagedTimeTableReuseIT { private int id; +protected String tableName; --- End diff -- No member variables for table names, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312842 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryIT.java --- @@ -148,7 +148,7 @@ public void testToDateOnString() throws Exception { // TODO: test more conversio } } - + --- End diff -- Please revert changes to this file since they only look whitespace related. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312676 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java --- @@ -61,7 +61,7 @@ protected static final String tenantId = getOrganizationId(); protected static final String ATABLE_INDEX_NAME = "ATABLE_IDX"; --- End diff -- Nevermind my comment since this class hasn't been converted yet. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312374 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryDatabaseMetaDataIT.java --- @@ -1066,7 +1065,7 @@ public void testAddPKColumn() throws Exception { public void testDropKVColumn() throws Exception { long ts = nextTimestamp(); String tenantId = getOrganizationId(); -initATableValues(tenantId, getDefaultSplits(tenantId), null, ts); +initATableValues("aTable", tenantId, getDefaultSplits(tenantId), null, ts, getUrl()); --- End diff -- Use static constant for "aTable" in BaseTest. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312396 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryDatabaseMetaDataIT.java --- @@ -1106,7 +1105,7 @@ public void testDropKVColumn() throws Exception { public void testDropPKColumn() throws Exception { long ts = nextTimestamp(); String tenantId = getOrganizationId(); -initATableValues(tenantId, getDefaultSplits(tenantId), null, ts); +initATableValues("aTable", tenantId, getDefaultSplits(tenantId), null, ts, getUrl()); --- End diff -- Use static constant for "aTable" in BaseTest. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74312230 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/PhoenixRuntimeIT.java --- @@ -95,53 +96,62 @@ private static Filter getUserTableAndViewsFilter() { } private void testGetTenantIdExpression(boolean isSalted) throws Exception { +//Have to metaData tables because BaseHBaseManagedTimeTableReuseIT doesn't delete them after each test case , and tenant list will create issues between test cases +deletePriorMetaData(HConstants.LATEST_TIMESTAMP, getUrl()); --- End diff -- Do we need this if we randomly generate the tenantIds? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74311348 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/LnLogFunctionEnd2EndIT.java --- @@ -33,10 +33,12 @@ /** * End to end tests for {@link LnFunction} and {@link LogFunction} */ -public class LnLogFunctionEnd2EndIT extends BaseHBaseManagedTimeIT { +public class LnLogFunctionEnd2EndIT extends BaseHBaseManagedTimeTableReuseIT { private static final String KEY = "key"; private static final double ZERO = 1e-9; +private String signedTableName; --- End diff -- No member variables for storing table names, please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74310790 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/HashJoinMoreIT.java --- @@ -43,7 +43,7 @@ import com.google.common.collect.Maps; -public class HashJoinMoreIT extends BaseHBaseManagedTimeIT { +public class HashJoinMoreIT extends BaseHBaseManagedTimeTableReuseIT { --- End diff -- It doesn't seem like this test was converted to use random table names. Please revert the changes to this file. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74310422 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/ExtendedQueryExecIT.java --- @@ -51,7 +51,7 @@ public void testToDateFunctionBind() throws Exception { Date date = new Date(1); String tenantId = getOrganizationId(); -initATableValues(tenantId, getDefaultSplits(tenantId),date, ts); +initATableValues("aTable", tenantId, getDefaultSplits(tenantId),date, ts, getUrl()); --- End diff -- If you need to pass the atable as name then use the static constant defined in BaseTest. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74310182 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/ExpFunctionEnd2EndIT.java --- @@ -34,21 +34,26 @@ /** * End to end tests for {@link ExpFunction} */ -public class ExpFunctionEnd2EndIT extends BaseHBaseManagedTimeIT { +public class ExpFunctionEnd2EndIT extends BaseHBaseManagedTimeTableReuseIT { private static final String KEY = "key"; private static final double ZERO = 1e-8; +private String signedTableName; --- End diff -- Same comment as before. Don't have table names as member variables as it makes running tests in parallel difficult. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74309672 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/DynamicColumnIT.java --- @@ -59,16 +59,18 @@ */ -public class DynamicColumnIT extends BaseHBaseManagedTimeIT { +public class DynamicColumnIT extends BaseHBaseManagedTimeTableReuseIT { private static final byte[] FAMILY_NAME_A = Bytes.toBytes(SchemaUtil.normalizeIdentifier("A")); private static final byte[] FAMILY_NAME_B = Bytes.toBytes(SchemaUtil.normalizeIdentifier("B")); +private String tableName = "TESTTBL"; --- End diff -- Shouldn't the name be generated by generateRandomString() ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74307945 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java --- @@ -18,20 +18,7 @@ package org.apache.phoenix.end2end; import static org.apache.phoenix.query.QueryConstants.MILLIS_IN_DAY; -import static org.apache.phoenix.util.TestUtil.ATABLE_NAME; --- End diff -- The import preferences don't look correct. For guidelines see the code conventions section here: http://phoenix.apache.org/contributing.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74308894 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/DerivedTableIT.java --- @@ -72,7 +72,7 @@ public DerivedTableIT(String[] indexDDL, String[] plans) { @Before public void initTable() throws Exception { ts = nextTimestamp(); -initATableValues(tenantId, getDefaultSplits(tenantId), null, ts); +initATableValues("aTable", tenantId, getDefaultSplits(tenantId), null, ts, getUrl()); --- End diff -- Don't think we need to pass "aTable" here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74307236 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/CSVCommonsLoaderIT.java --- @@ -45,15 +45,15 @@ import org.apache.phoenix.util.PhoenixRuntime; import org.junit.Test; -public class CSVCommonsLoaderIT extends BaseHBaseManagedTimeIT { +public class CSVCommonsLoaderIT extends BaseHBaseManagedTimeTableReuseIT { private static final String DATATYPE_TABLE = "DATATYPE"; private static final String DATATYPES_CSV_VALUES = "CKEY, CVARCHAR, CCHAR, CINTEGER, CDECIMAL, CUNSIGNED_INT, CBOOLEAN, CBIGINT, CUNSIGNED_LONG, CTIME, CDATE\n" + "KEY1,A,A,2147483647,1.1,0,TRUE,9223372036854775807,0,1990-12-31 10:59:59,1999-12-31 23:59:59\n" + "KEY2,B,B,-2147483648,-1.1,2147483647,FALSE,-9223372036854775808,9223372036854775807,2000-01-01 00:00:01,2012-02-29 23:59:59\n" + "KEY3,,\n"; -private static final String STOCK_TABLE = "STOCK_SYMBOL"; -private static final String STOCK_TABLE_MULTI = "STOCK_SYMBOL_MULTI"; +//private static final String stockTableName = "STOCK_SYMBOL"; --- End diff -- remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74308491 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java --- @@ -57,12 +44,13 @@ import org.junit.Test; -public class DateTimeIT extends BaseHBaseManagedTimeIT { +public class DateTimeIT extends BaseHBaseManagedTimeTableReuseIT { protected Connection conn; protected Date date; protected static final String tenantId = getOrganizationId(); protected final static String ROW10 = "00D123122312312"; +protected String tableName; --- End diff -- Don't have tableName as a member variable. Instead each test case can create their own table. This will be critical once we decide on parallelizing test methods within a test class. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74306317 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java --- @@ -61,7 +61,7 @@ protected static final String tenantId = getOrganizationId(); protected static final String ATABLE_INDEX_NAME = "ATABLE_IDX"; --- End diff -- Shouldn't the index name also be a random string? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74306178 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseQueryIT.java --- @@ -90,7 +90,8 @@ public BaseQueryIT(String indexDDL) { @Before public void initTable() throws Exception { ts = nextTimestamp(); -initATableValues(tenantId, getDefaultSplits(tenantId), date=new Date(System.currentTimeMillis()), ts); + +initATableValues("aTable", tenantId, getDefaultSplits(tenantId), date=new Date(System.currentTimeMillis()), ts, getUrl()); --- End diff -- Is there a need to pass "aTable" here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74305946 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseHBaseManagedTimeTableReuseIT.java --- @@ -47,6 +48,10 @@ @NotThreadSafe @Category(HBaseManagedTimeTableReuseTest.class) public class BaseHBaseManagedTimeTableReuseIT extends BaseTest { +protected static Configuration getTestClusterConfig() { --- End diff -- I don't see this method being used anywhere. Please remove if it isn't needed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74304816 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java --- @@ -1052,16 +1057,20 @@ private static void deletePriorSequences(long ts, Connection globalConn) throws } rs.close(); } - + protected static void initSumDoubleValues(byte[][] splits, String url) throws Exception { -ensureTableCreated(url, "SumDoubleTest", splits); +initSumDoubleValues("SumDoubleTest", splits, url); +} + +protected static void initSumDoubleValues(String tableName, byte[][] splits, String url) throws Exception { +ensureTableCreated(url, tableName, "SumDoubleTest", splits); --- End diff -- Minor nit: Define a static constant for SumDoubleTest like other table ddl types. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r74304218 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java --- @@ -274,36 +285,15 @@ public void testTableAddColumnsDifferentClient() throws Exception { testAddColumns(false); } -public void testCreateTableDropColumns() throws Exception { --- End diff -- Why was this test case removed? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r73924877 --- Diff: pom.xml --- @@ -226,6 +226,7 @@ maven-failsafe-plugin ${maven-failsafe-plugin.version} + --- End diff -- Revert changes to this file since it has only white space changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r73924704 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java --- @@ -2063,5 +2080,35 @@ protected static void populateMultiCFTestTable(String tableName, Date date) thro } finally { conn.close(); } -} +} + +protected static void verifySequence(String tenantID, String sequenceName, String sequenceSchemaName, boolean exists) throws SQLException { + +PhoenixConnection phxConn = DriverManager.getConnection(getUrl()).unwrap(PhoenixConnection.class); +String ddl = "SELECT " ++ PhoenixDatabaseMetaData.TENANT_ID + "," ++ PhoenixDatabaseMetaData.SEQUENCE_SCHEMA + "," ++ PhoenixDatabaseMetaData.SEQUENCE_NAME ++ " FROM " + PhoenixDatabaseMetaData.SYSTEM_SEQUENCE ++ " WHERE "; + +ddl += " TENANT_ID " + ((tenantID == null ) ? "IS NULL " : " = '" + tenantID + "'"); +ddl += " AND SEQUENCE_NAME " + ((sequenceName == null) ? "IS NULL " : " = '" + sequenceName + "'"); +ddl += " AND SEQUENCE_SCHEMA " + ((sequenceSchemaName == null) ? "IS NULL " : " = '" + sequenceSchemaName + "'" ); + +ResultSet rs = phxConn.createStatement().executeQuery(ddl); +//boolean res = --- End diff -- Remove commented code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request #189: PHOENIX-3036 Modify phoenix IT tests to extend Ba...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/189#discussion_r73924614 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java --- @@ -2063,5 +2080,35 @@ protected static void populateMultiCFTestTable(String tableName, Date date) thro } finally { conn.close(); } -} +} + +protected static void verifySequence(String tenantID, String sequenceName, String sequenceSchemaName, boolean exists) throws SQLException { + +PhoenixConnection phxConn = DriverManager.getConnection(getUrl()).unwrap(PhoenixConnection.class); +String ddl = "SELECT " ++ PhoenixDatabaseMetaData.TENANT_ID + "," ++ PhoenixDatabaseMetaData.SEQUENCE_SCHEMA + "," ++ PhoenixDatabaseMetaData.SEQUENCE_NAME ++ " FROM " + PhoenixDatabaseMetaData.SYSTEM_SEQUENCE ++ " WHERE "; + +ddl += " TENANT_ID " + ((tenantID == null ) ? "IS NULL " : " = '" + tenantID + "'"); +ddl += " AND SEQUENCE_NAME " + ((sequenceName == null) ? "IS NULL " : " = '" + sequenceName + "'"); +ddl += " AND SEQUENCE_SCHEMA " + ((sequenceSchemaName == null) ? "IS NULL " : " = '" + sequenceSchemaName + "'" ); + +ResultSet rs = phxConn.createStatement().executeQuery(ddl); +//boolean res = +while(rs.next()){ +String ten = rs.getString("TENANT_ID"); +String seqN = rs.getString("SEQUENCE_SCHEMA"); +String seqaN = rs.getString("SEQUENCE_NAME"); +String seqNam = rs.getString("SEQUENCE_SCHEMA"); +} +/*if(exists) { --- End diff -- This shouldn't be commented out, I think. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/153#issuecomment-209605592 This looks great now @ankitsinghal. I am +1 once you have addressed the last review comments. Thanks a lot for your patience and diligence! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r59608279 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -2387,8 +2452,27 @@ public Void call() throws Exception { logger.info("Update of SYSTEM.CATALOG complete"); clearCache(); } - + +if (currentServerSideTableTimeStamp < MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_8_0) { +// Add these columns one at a time, each with different timestamps so that if folks --- End diff -- This comment is just a copy paste of the comment in the previous if block. It would make more sense to move this comment to above if (currentServerSideTableTimeStamp < MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_6_0) {} block as it applies to all the upgrades we will do in future. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r59607658 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java --- @@ -77,6 +79,7 @@ public SQLException newException(SQLExceptionInfo info) { MISSING_MAX_LENGTH(207, "22004", "Max length must be specified for type."), NONPOSITIVE_MAX_LENGTH(208, "22006", "Max length must have a positive length for type."), DECIMAL_PRECISION_OUT_OF_RANGE(209, "22003", "Decimal precision outside of range. Should be within 1 and " + PDataType.MAX_PRECISION + "."), +ILLEGAL_OPERATION(210, "22010", "Illegal Operation."), --- End diff -- Change this to something like this: CREATE_SCHEMA_NOT_ALLOWED(210, "22010", "Cannot create schema because config " + QueryServicesOptions. IS_NAMESPACE_MAPPING_ENABLED " + " for enabling name space mapping isn't enabled.") --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r59607039 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/IllegalOperationException.java --- @@ -0,0 +1,45 @@ +/* + * 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.schema; + +import org.apache.phoenix.exception.SQLExceptionCode; +import org.apache.phoenix.exception.SQLExceptionInfo; + +/** + * + * Exception thrown when any illegal operation is performed. + * + * + * @since 180 + */ +public class IllegalOperationException extends RuntimeException { --- End diff -- Not sure if you need a new exception for this. Can you not just throw a SQLException instead? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/153#issuecomment-209161997 Thanks for the updated request, @ankitsinghal. Almost there! Can you tell me more about what would happen in the following scenario if the name space feature is disabled in config: CREATE SCHEMA S; USE SCHEMA S; CREATE TABLE T; Will the table T be created in the namespace S? If the feature is disabled, it shouldn't be. Would you mind adding a test for this if it hasn't been already? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r59307776 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/PTableKey.java --- @@ -26,7 +28,8 @@ public PTableKey(PName tenantId, String name) { Preconditions.checkNotNull(name); this.tenantId = tenantId; -this.name = name; +this.name = !name.contains(QueryConstants.NAMESPACE_SEPARATOR) ? name --- End diff -- I think we could potentially mask some bug or have it fail later than it should if we do this. We should remove this check altogether. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/153#issuecomment-208573524 @ankitsinghal - thanks for adding the upgrade util test. I have a question/suggestion: we should probably disallow users to execute create and use schema statements if the name space feature isn't enabled. I see that you are currently failing the upgrade if the name space feature isn't enabled which is good. To be consistent we should do the same for create and use schema statements. Another thing to think would be whether we should allow setting schema property on connection if name space feature is disabled. I lean towards not allowing it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-207594418 @churrodog - can you please post a rebased patch on the jira. The current patch doesn't apply on master. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-207591521 Thanks @churrodog. Nice work! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r59082461 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java --- @@ -265,12 +332,18 @@ public SingleTableColumnResolver(PhoenixConnection connection, NamedTableNode ta if (def.getColumnDefName().getFamilyName() != null) { families.add(new PColumnFamilyImpl(PNameFactory.newName(def.getColumnDefName().getFamilyName()),Collections.emptyList())); } - } - Long scn = connection.getSCN(); - PTable theTable = new PTableImpl(connection.getTenantId(), table.getName().getSchemaName(), table.getName().getTableName(), scn == null ? HConstants.LATEST_TIMESTAMP : scn, families); +} +Long scn = connection.getSCN(); +String schema = table.getName().getSchemaName(); +if (connection.getSchema() != null) { --- End diff -- Makes sense. Thanks for the clarification @ankitsinghal --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58990926 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/tx/TxCheckpointIT.java --- @@ -96,6 +97,7 @@ public void testUpsertSelectDoesntSeeUpsertedData() throws Exception { Connection conn = DriverManager.getConnection(getUrl(), props); conn.setAutoCommit(true); conn.createStatement().execute("CREATE SEQUENCE "+seqName); +BaseTest.createSchema(getUrl(), fullTableName, null); --- End diff -- Is this change needed? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58990747 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java --- @@ -632,6 +645,16 @@ public static ExecutionCommand parseArgs(String[] args) { return execCmd; } +private static String validateTableName(String tableName) { --- End diff -- Looking at this method it seems like table names can't have : in them. Does the change in PTableKey.java still make sense then? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58990594 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/stats/StatisticsCollectorFactory.java --- @@ -63,6 +64,10 @@ public static StatisticsCollector createStatisticsCollector( DISABLE_STATS.add(TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_FUNCTION_NAME)); DISABLE_STATS.add(TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_SEQUENCE_NAME)); DISABLE_STATS.add(TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_STATS_NAME)); + DISABLE_STATS.add(SchemaUtil.getPhysicalTableName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES,true)); --- End diff -- Same as in the other place. Is name space mapping always true? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58990540 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/PTableKey.java --- @@ -26,7 +28,8 @@ public PTableKey(PName tenantId, String name) { Preconditions.checkNotNull(name); this.tenantId = tenantId; -this.name = name; +this.name = !name.contains(QueryConstants.NAMESPACE_SEPARATOR) ? name --- End diff -- So if the table name was A:B, the key would become A.B. Can you add some test case around this and make sure creating, query, upserting to a table named like this works fine. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58989689 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java --- @@ -631,7 +664,7 @@ private boolean addIndexesFromPhysicalTable(MetaDataMutationResult result, Long if (view.getType() != PTableType.VIEW || view.getViewType() == ViewType.MAPPED) { return false; } -String physicalName = view.getPhysicalName().getString(); +String physicalName = view.getPhysicalNames().get(0).toString(); --- End diff -- Please undo this change as we still don't allow creating views over multiple tables. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58989498 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/QueryServicesOptions.java --- @@ -133,11 +135,13 @@ // latency and client-side spooling/buffering. Smaller means less initial // latency and less parallelization. public static final long DEFAULT_SCAN_RESULT_CHUNK_SIZE = 2999; +public static final boolean DEFAULT_IS_NAMESPACE_MAPPING_ENABLED = false; +public static final boolean DEFAULT_IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE = false; // // Spillable GroupBy - SPGBY prefix // -// Enable / disable spillable group by +// Enable / disablfalsellable group by --- End diff -- Please undo. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58989392 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -2387,8 +2452,27 @@ public Void call() throws Exception { logger.info("Update of SYSTEM.CATALOG complete"); clearCache(); } - + +if (currentServerSideTableTimeStamp < MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_8_0) { +// Add these columns one at a time, each with different timestamps so that if folks --- End diff -- Please remove this comment as it doesn't make sense here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58988872 --- Diff: phoenix-core/src/main/java/org/apache/hadoop/hbase/ipc/controller/MetadataRpcController.java --- @@ -36,7 +37,16 @@ .add(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME) .add(PhoenixDatabaseMetaData.SYSTEM_STATS_NAME) .add(PhoenixDatabaseMetaData.SYSTEM_SEQUENCE_NAME) - .add(PhoenixDatabaseMetaData.SYSTEM_FUNCTION_NAME).build(); + .add(PhoenixDatabaseMetaData.SYSTEM_FUNCTION_NAME) + .add(SchemaUtil.getPhysicalTableName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, true) --- End diff -- is the namespace mapping enabled always for system tables (which is what I am inferring from true being passed from isNamespaceMapped) ? Looking at QueryServicesOptions.java, looks like it isn't. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58987883 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryDatabaseMetaDataIT.java --- @@ -190,14 +190,8 @@ public void testSchemaMetadataScan() throws SQLException { rs = dbmd.getSchemas(null, null); assertTrue(rs.next()); -assertEquals(rs.getString("TABLE_SCHEM"),null); --- End diff -- Is there a reason why these lines are removed? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58986201 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java --- @@ -265,12 +332,18 @@ public SingleTableColumnResolver(PhoenixConnection connection, NamedTableNode ta if (def.getColumnDefName().getFamilyName() != null) { families.add(new PColumnFamilyImpl(PNameFactory.newName(def.getColumnDefName().getFamilyName()),Collections.emptyList())); } - } - Long scn = connection.getSCN(); - PTable theTable = new PTableImpl(connection.getTenantId(), table.getName().getSchemaName(), table.getName().getTableName(), scn == null ? HConstants.LATEST_TIMESTAMP : scn, families); +} +Long scn = connection.getSCN(); +String schema = table.getName().getSchemaName(); +if (connection.getSchema() != null) { --- End diff -- I meant if the connection has a schema present, and the table's schema is different from the schema property in the connection, IMHO we should probably throw an error like SchemaMismatchException. I could think of a few combinations that we should validate and test: is namespace mapped connection schema yesnull yesdifferent yessame no null no non-null You could probably add a method in SchemaUtil to do the above checks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on the pull request: https://github.com/apache/phoenix/pull/158#issuecomment-206989941 The pull request looks great @churrodog. Just a couple of minor nits. I will get this committed once you have the changes in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/158#discussion_r58903054 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/memory/GlobalMemoryManager.java --- @@ -94,6 +85,20 @@ private long allocateBytes(long minBytes, long reqBytes) { return nBytes; } +@VisibleForTesting void waitForBytesToFree(long minBytes, long startTimeMs) { --- End diff -- Minor nit: Missing line carriage after @VisibleForTesting --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/158#discussion_r58902469 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java --- @@ -69,35 +76,38 @@ private static void sleepFor(long time) { } @Test -public void testWaitForMemoryAvailable() { -final GlobalMemoryManager gmm = new GlobalMemoryManager(100,8000); +public void testWaitForMemoryAvailable() throws Exception { +final GlobalMemoryManager gmm = spy(new GlobalMemoryManager(100, 80)); final ChildMemoryManager rmm1 = new ChildMemoryManager(gmm,100); final ChildMemoryManager rmm2 = new ChildMemoryManager(gmm,100); +final CountDownLatch latch = new CountDownLatch(2); Thread t1 = new Thread() { @Override public void run() { MemoryChunk c1 = rmm1.allocate(50); MemoryChunk c2 = rmm1.allocate(50); -sleepFor(4000); +sleepFor(40); c1.close(); -sleepFor(2000); +sleepFor(20); c2.close(); +latch.countDown(); } }; Thread t2 = new Thread() { @Override public void run() { -sleepFor(2000); +sleepFor(20); // Will require waiting for a bit of time before t1 frees the requested memory -long startTime = System.currentTimeMillis(); +Stopwatch watch = new Stopwatch().start(); --- End diff -- Actually, I don't see this watch being used anywhere. Make sure you have the phoenix eclipse preferences imported. This should have been flagged as an unused variable warning. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-2822 - Tests that extend BaseHBaseMa...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/158#discussion_r58902347 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/memory/MemoryManagerTest.java --- @@ -19,19 +19,26 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.spy; +import com.google.common.base.Stopwatch; --- End diff -- Don't use the guava StopWatch here. We have run into conflicts before because of mismatches in guava versions. You can use the PhoenixStopWatch class here instead. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58597202 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); --- End diff -- Does disabling and deleting the srcTable here also delete the snapshot you created above. If not, would it make sense to delete the snapshot? This is also making me think of failure scenarios. What if any of the steps here fails? Do we restore the table using the snapshot? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58594903 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, --- End diff -- Is it possible to add any tests around the upgrade code here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58594707 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( +"May not specify the TENANT_ID_ATTRIB property when upgrading"); } +try (HBaseAdmin admin = conn.getQueryServices().getAdmin(); +HTableInterface metatable = conn.getQueryServices() +.getTable(SchemaUtil + .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, readOnlyProps) +.getName());) { +String tableName = SchemaUtil.normalizeIdentifier(srcTable); +String schemaName = SchemaUtil.getSchemaNameFromFullName(tableName); + +// Upgrade is not required if schemaName is not present. +if (schemaName.equals("")) { throw new IllegalArgumentException("Table doesn't have schema name"); } + +// Confirm table is not already upgraded +PTable table = PhoenixRuntime.getTable(conn, tableName); +if (table.isNamespaceMapped()) { throw new IllegalArgumentException("Table is already upgraded"); } +conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName); +String newPhysicalTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(table.getPhysicalName().getString(), readOnlyProps).getNameAsString()); + +// Upgrade the data or main table +UpgradeUtil.mapTableToNamespace(admin, metatable, tableName, newPhysicalTablename, readOnl
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58469369 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( +"May not specify the TENANT_ID_ATTRIB property when upgrading"); } +try (HBaseAdmin admin = conn.getQueryServices().getAdmin(); +HTableInterface metatable = conn.getQueryServices() +.getTable(SchemaUtil + .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, readOnlyProps) +.getName());) { +String tableName = SchemaUtil.normalizeIdentifier(srcTable); +String schemaName = SchemaUtil.getSchemaNameFromFullName(tableName); + +// Upgrade is not required if schemaName is not present. +if (schemaName.equals("")) { throw new IllegalArgumentException("Table doesn't have schema name"); } + +// Confirm table is not already upgraded +PTable table = PhoenixRuntime.getTable(conn, tableName); +if (table.isNamespaceMapped()) { throw new IllegalArgumentException("Table is already upgraded"); } +conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName); +String newPhysicalTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(table.getPhysicalName().getString(), readOnlyProps).getNameAsString()); + +// Upgrade the data or main table +UpgradeUtil.mapTableToNamespace(admin, metatable, tableName, newPhysicalTablename, readOnlyProps,
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58469290 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( +"May not specify the TENANT_ID_ATTRIB property when upgrading"); } +try (HBaseAdmin admin = conn.getQueryServices().getAdmin(); +HTableInterface metatable = conn.getQueryServices() +.getTable(SchemaUtil + .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, readOnlyProps) +.getName());) { +String tableName = SchemaUtil.normalizeIdentifier(srcTable); +String schemaName = SchemaUtil.getSchemaNameFromFullName(tableName); + +// Upgrade is not required if schemaName is not present. +if (schemaName.equals("")) { throw new IllegalArgumentException("Table doesn't have schema name"); } + +// Confirm table is not already upgraded +PTable table = PhoenixRuntime.getTable(conn, tableName); +if (table.isNamespaceMapped()) { throw new IllegalArgumentException("Table is already upgraded"); } +conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName); +String newPhysicalTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(table.getPhysicalName().getString(), readOnlyProps).getNameAsString()); + +// Upgrade the data or main table +UpgradeUtil.mapTableToNamespace(admin, metatable, tableName, newPhysicalTablename, readOnlyProps,
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58469125 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( +"May not specify the TENANT_ID_ATTRIB property when upgrading"); } +try (HBaseAdmin admin = conn.getQueryServices().getAdmin(); +HTableInterface metatable = conn.getQueryServices() +.getTable(SchemaUtil + .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, readOnlyProps) +.getName());) { +String tableName = SchemaUtil.normalizeIdentifier(srcTable); +String schemaName = SchemaUtil.getSchemaNameFromFullName(tableName); + +// Upgrade is not required if schemaName is not present. +if (schemaName.equals("")) { throw new IllegalArgumentException("Table doesn't have schema name"); } + +// Confirm table is not already upgraded +PTable table = PhoenixRuntime.getTable(conn, tableName); +if (table.isNamespaceMapped()) { throw new IllegalArgumentException("Table is already upgraded"); } +conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName); +String newPhysicalTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(table.getPhysicalName().getString(), readOnlyProps).getNameAsString()); + +// Upgrade the data or main table +UpgradeUtil.mapTableToNamespace(admin, metatable, tableName, newPhysicalTablename, readOnlyProps,
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58469055 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( +"May not specify the TENANT_ID_ATTRIB property when upgrading"); } +try (HBaseAdmin admin = conn.getQueryServices().getAdmin(); +HTableInterface metatable = conn.getQueryServices() +.getTable(SchemaUtil + .getPhysicalName(PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME_BYTES, readOnlyProps) +.getName());) { +String tableName = SchemaUtil.normalizeIdentifier(srcTable); +String schemaName = SchemaUtil.getSchemaNameFromFullName(tableName); + +// Upgrade is not required if schemaName is not present. +if (schemaName.equals("")) { throw new IllegalArgumentException("Table doesn't have schema name"); } + +// Confirm table is not already upgraded +PTable table = PhoenixRuntime.getTable(conn, tableName); +if (table.isNamespaceMapped()) { throw new IllegalArgumentException("Table is already upgraded"); } +conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName); +String newPhysicalTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(table.getPhysicalName().getString(), readOnlyProps).getNameAsString()); + +// Upgrade the data or main table +UpgradeUtil.mapTableToNamespace(admin, metatable, tableName, newPhysicalTablename, readOnlyProps,
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58468877 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { +phoenixTableName = srcTableName; +} +Put put = new Put(SchemaUtil.getTableKey(null, SchemaUtil.getSchemaNameFromFullName(phoenixTableName), +SchemaUtil.getTableNameFromFullName(phoenixTableName)), ts); +put.addColumn(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES, PhoenixDatabaseMetaData.IS_NAMESPACE_MAPPED_BYTES, +PBoolean.INSTANCE.toBytes(Boolean.TRUE)); +metatable.put(put); +} + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String tableName, +ReadOnlyProps props, Long ts) throws SnapshotCreationException, IllegalArgumentException, IOException, +InterruptedException, SQLException { +String destTablename = SchemaUtil + .normalizeIdentifier(SchemaUtil.getPhysicalTableName(tableName, props).getNameAsString()); +mapTableToNamespace(admin, metatable, tableName, destTablename, props, ts, null, PTableType.TABLE); +} + +public static void upgradeTable(PhoenixConnection conn, String srcTable) throws SQLException, +SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException { +ReadOnlyProps readOnlyProps = conn.getQueryServices().getProps(); +if (conn.getClientInfo(PhoenixRuntime.TENANT_ID_ATTRIB) != null) { throw new SQLException( --- End diff -- What if the connection has schema property set? Should we report an error. Looks like currently we are ignoring it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58468394 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( +SchemaUtil.isSystemTable(srcTableName.getBytes()) ? PTableType.SYSTEM : null, +props)) { throw new IllegalArgumentException(SchemaUtil.isSystemTable(srcTableName.getBytes()) +? "For system table " + QueryServices.IS_SYSTEM_TABLE_MAPPED_TO_NAMESPACE ++ " also needs to be enabled along with " + QueryServices.IS_NAMESPACE_MAPPING_ENABLED +: QueryServices.IS_NAMESPACE_MAPPING_ENABLED + " is not enabled"); } + +if (PTableType.TABLE.equals(pTableType) || PTableType.INDEX.equals(pTableType)) { +admin.snapshot(srcTableName, srcTableName); +admin.cloneSnapshot(srcTableName.getBytes(), destTableName.getBytes()); +admin.disableTable(srcTableName); +admin.deleteTable(srcTableName); +} +if (phoenixTableName == null) { --- End diff -- This looks a bit hacky/unclear to me. When can phoenixTableName be null here? And when is it ok to use the srcTableName. At a minimum some method comments would help. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58468008 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java --- @@ -1279,4 +1304,129 @@ public static boolean truncateStats(HTableInterface metaTable, HTableInterface s } return false; } + +public static void mapTableToNamespace(HBaseAdmin admin, HTableInterface metatable, String srcTableName, +String destTableName, ReadOnlyProps props, Long ts, String phoenixTableName, PTableType pTableType) +throws SnapshotCreationException, IllegalArgumentException, IOException, InterruptedException, +SQLException { +srcTableName = SchemaUtil.normalizeIdentifier(srcTableName); +if (!SchemaUtil.isNamespaceMappingEnabled( --- End diff -- Can you not just pass the pTableType argument here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58467711 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java --- @@ -897,4 +941,86 @@ public static boolean hasRowTimestampColumn(PTable table) { PName schemaName = dataTable.getSchemaName(); return getTableKey(tenantId == null ? ByteUtil.EMPTY_BYTE_ARRAY : tenantId.getBytes(), schemaName == null ? ByteUtil.EMPTY_BYTE_ARRAY : schemaName.getBytes(), dataTable.getTableName().getBytes()); } + +public static byte[] getSchemaKey(String schemaName) { +return SchemaUtil.getTableKey(null, schemaName, MetaDataClient.EMPTY_TABLE); +} + +public static PName getPhysicalHBaseTableName(PName pName, boolean isNamespaceMapped, PTableType type) { +return getPhysicalHBaseTableName(pName.toString(), isNamespaceMapped, type); +} + +public static PName getPhysicalHBaseTableName(byte[] tableName, boolean isNamespaceMapped, PTableType type) { +return getPhysicalHBaseTableName(Bytes.toString(tableName), isNamespaceMapped, type); +} + +public static TableName getPhysicalTableName(String fullTableName, ReadOnlyProps readOnlyProps) { +return getPhysicalName(Bytes.toBytes(fullTableName), readOnlyProps); +} + +public static TableName getPhysicalTableName(byte[] fullTableName, Configuration conf) { +return getPhysicalTableName(fullTableName, isNamespaceMappingEnabled( +isSystemTable(fullTableName) ? PTableType.SYSTEM : null, new ReadOnlyProps(conf.iterator(; +} + +public static TableName getPhysicalName(byte[] fullTableName, ReadOnlyProps readOnlyProps) { +return getPhysicalTableName(fullTableName, +isNamespaceMappingEnabled(isSystemTable(fullTableName) ? PTableType.SYSTEM : null, readOnlyProps)); +} + +public static TableName getPhysicalTableName(byte[] fullTableName, boolean isNamespaceMappingEnabled) { +if (!isNamespaceMappingEnabled) { return TableName.valueOf(fullTableName); } +String tableName = getTableNameFromFullName(fullTableName); +String schemaName = getSchemaNameFromFullName(fullTableName); +return TableName.valueOf(schemaName, tableName); +} + +public static String getSchemaNameFromHBaseFullName(byte[] tableName, ReadOnlyProps props) { +if (tableName == null) { return null; } +int index = isNamespaceMappingEnabled(null, props) ? indexOf(tableName, QueryConstants.NAMESPACE_SEPARATOR_BYTE) +: indexOf(tableName, QueryConstants.NAME_SEPARATOR_BYTE); +if (index < 0) { return StringUtil.EMPTY_STRING; } +return Bytes.toString(tableName, 0, index); +} + +public static PName getPhysicalHBaseTableName(String tableName, boolean isNamespaceMapped, PTableType type) { +if (!isNamespaceMapped) { return PNameFactory.newName(tableName); } +return PNameFactory +.newName(tableName.replace(QueryConstants.NAME_SEPARATOR, QueryConstants.NAMESPACE_SEPARATOR)); +} + +public static boolean isSchemaCheckRequired(PTableType tableType, ReadOnlyProps props) { +if (PTableType.TABLE.equals(tableType) && isNamespaceMappingEnabled(tableType, props)) { return true; } --- End diff -- nit: how about return PTableType.TABLE.equals(tableType) && isNamespaceMappingEnabled(tableType, props); --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58467374 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java --- @@ -897,4 +941,86 @@ public static boolean hasRowTimestampColumn(PTable table) { PName schemaName = dataTable.getSchemaName(); return getTableKey(tenantId == null ? ByteUtil.EMPTY_BYTE_ARRAY : tenantId.getBytes(), schemaName == null ? ByteUtil.EMPTY_BYTE_ARRAY : schemaName.getBytes(), dataTable.getTableName().getBytes()); } + +public static byte[] getSchemaKey(String schemaName) { +return SchemaUtil.getTableKey(null, schemaName, MetaDataClient.EMPTY_TABLE); +} + +public static PName getPhysicalHBaseTableName(PName pName, boolean isNamespaceMapped, PTableType type) { +return getPhysicalHBaseTableName(pName.toString(), isNamespaceMapped, type); +} + +public static PName getPhysicalHBaseTableName(byte[] tableName, boolean isNamespaceMapped, PTableType type) { +return getPhysicalHBaseTableName(Bytes.toString(tableName), isNamespaceMapped, type); +} + +public static TableName getPhysicalTableName(String fullTableName, ReadOnlyProps readOnlyProps) { +return getPhysicalName(Bytes.toBytes(fullTableName), readOnlyProps); +} + +public static TableName getPhysicalTableName(byte[] fullTableName, Configuration conf) { +return getPhysicalTableName(fullTableName, isNamespaceMappingEnabled( +isSystemTable(fullTableName) ? PTableType.SYSTEM : null, new ReadOnlyProps(conf.iterator(; +} + +public static TableName getPhysicalName(byte[] fullTableName, ReadOnlyProps readOnlyProps) { +return getPhysicalTableName(fullTableName, +isNamespaceMappingEnabled(isSystemTable(fullTableName) ? PTableType.SYSTEM : null, readOnlyProps)); +} + +public static TableName getPhysicalTableName(byte[] fullTableName, boolean isNamespaceMappingEnabled) { +if (!isNamespaceMappingEnabled) { return TableName.valueOf(fullTableName); } +String tableName = getTableNameFromFullName(fullTableName); +String schemaName = getSchemaNameFromFullName(fullTableName); +return TableName.valueOf(schemaName, tableName); +} + +public static String getSchemaNameFromHBaseFullName(byte[] tableName, ReadOnlyProps props) { +if (tableName == null) { return null; } +int index = isNamespaceMappingEnabled(null, props) ? indexOf(tableName, QueryConstants.NAMESPACE_SEPARATOR_BYTE) +: indexOf(tableName, QueryConstants.NAME_SEPARATOR_BYTE); +if (index < 0) { return StringUtil.EMPTY_STRING; } +return Bytes.toString(tableName, 0, index); +} + +public static PName getPhysicalHBaseTableName(String tableName, boolean isNamespaceMapped, PTableType type) { +if (!isNamespaceMapped) { return PNameFactory.newName(tableName); } +return PNameFactory +.newName(tableName.replace(QueryConstants.NAME_SEPARATOR, QueryConstants.NAMESPACE_SEPARATOR)); +} + +public static boolean isSchemaCheckRequired(PTableType tableType, ReadOnlyProps props) { +if (PTableType.TABLE.equals(tableType) && isNamespaceMappingEnabled(tableType, props)) { return true; } +return false; +} + +public static boolean isNamespaceMappingEnabled(PTableType type, ReadOnlyProps readOnlyProps) { +return readOnlyProps.getBoolean(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, +QueryServicesOptions.DEFAULT_IS_NAMESPACE_MAPPING_ENABLED) +&& (type == null || !PTableType.SYSTEM.equals(type) --- End diff -- Can the type be null here. If not, I would annotate the argument as @Nonnull and have a Preconditions.checkNotNull(type) check at the beginning of the method. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58465875 --- Diff: phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java --- @@ -825,9 +825,25 @@ protected static void ensureTableCreated(String url, String tableName, Long ts) protected static void ensureTableCreated(String url, String tableName, byte[][] splits, Long ts) throws SQLException { String ddl = tableDDLMap.get(tableName); +createSchema(url,tableName, ts); createTestTable(url, ddl, splits, ts); } +public static void createSchema(String url, String tableName, Long ts) throws SQLException { +if (tableName.contains(".")) { --- End diff -- Use SchemaUtil.getSchemaNameFromFullName and related methods here to parse the schema name and table name. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58465248 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java --- @@ -265,12 +332,18 @@ public SingleTableColumnResolver(PhoenixConnection connection, NamedTableNode ta if (def.getColumnDefName().getFamilyName() != null) { families.add(new PColumnFamilyImpl(PNameFactory.newName(def.getColumnDefName().getFamilyName()),Collections.emptyList())); } - } - Long scn = connection.getSCN(); - PTable theTable = new PTableImpl(connection.getTenantId(), table.getName().getSchemaName(), table.getName().getTableName(), scn == null ? HConstants.LATEST_TIMESTAMP : scn, families); +} +Long scn = connection.getSCN(); +String schema = table.getName().getSchemaName(); +if (connection.getSchema() != null) { --- End diff -- Not sure if functionally this is the right thing to do here. I would expect Phoenix to thrown an exception when the connection's setting of schema/namespace is different from the the table's namespace/schema. Or is my understanding not correct here? Would be good to add a test case for this too in UseSchemaIT. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58118836 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -3201,4 +3367,155 @@ private MetaDataMutationResult doDropFunction(long clientTimeStamp, List<byte[]> return new MetaDataMutationResult(MutationCode.FUNCTION_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); } + +@Override +public void createSchema(RpcController controller, CreateSchemaRequest request, +RpcCallback done) { +MetaDataResponse.Builder builder = MetaDataResponse.newBuilder(); +String schemaName = null; +try { +List schemaMutations = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(schemaMutations); + +byte[] lockKey = m.getRow(); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region); +if (result != null) { +done.run(MetaDataMutationResult.toProto(result)); +return; +} +List locks = Lists.newArrayList(); +long clientTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); +try { +acquireLock(region, lockKey, locks); +// Get as of latest timestamp so we can detect if we have a +// newer function that already +// exists without making an additional query +ImmutableBytesPtr cacheKey = new ImmutableBytesPtr(lockKey); +PSchema schema = loadSchema(env, lockKey, cacheKey, clientTimeStamp, clientTimeStamp); +if (schema != null) { +if (schema.getTimeStamp() < clientTimeStamp) { + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_ALREADY_EXISTS); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} else { + builder.setReturnCode(MetaDataProtos.MutationCode.NEWER_SCHEMA_FOUND); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} +} +region.mutateRowsWithLocks(schemaMutations, Collections.<byte[]> emptySet(), HConstants.NO_NONCE, +HConstants.NO_NONCE); + +// Invalidate the cache - the next getTable call will add it +// TODO: consider loading the table that was just created here, +// patching up the parent table, and updating the cache +Cache<ImmutableBytesPtr, PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env) +.getMetaDataCache(); +if (cacheKey != null) { +metaDataCache.invalidate(cacheKey); +} + +// Get timeStamp from mutations - the above method sets it if +// it's unset +long currentTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_NOT_FOUND); +builder.setMutationTime(currentTimeStamp); +done.run(builder.build()); +return; +} finally { +region.releaseRowLocks(locks); +} +} catch (Throwable t) { +logger.error("createFunction failed", t); +ProtobufUtil.setControllerException(controller, ServerUtil.createIOException(schemaName, t)); +} +} + +@Override +public void dropSchema(RpcController controller, DropSchemaRequest request, RpcCallback done) { +String schemaName = null; +try { +List schemaMetaData = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +byte[] lockKey = SchemaUtil.getSchemaKey(schemaName); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region);
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58118513 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -3201,4 +3367,155 @@ private MetaDataMutationResult doDropFunction(long clientTimeStamp, List<byte[]> return new MetaDataMutationResult(MutationCode.FUNCTION_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); } + +@Override +public void createSchema(RpcController controller, CreateSchemaRequest request, +RpcCallback done) { +MetaDataResponse.Builder builder = MetaDataResponse.newBuilder(); +String schemaName = null; +try { +List schemaMutations = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(schemaMutations); + +byte[] lockKey = m.getRow(); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region); +if (result != null) { +done.run(MetaDataMutationResult.toProto(result)); +return; +} +List locks = Lists.newArrayList(); +long clientTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); +try { +acquireLock(region, lockKey, locks); +// Get as of latest timestamp so we can detect if we have a +// newer function that already +// exists without making an additional query +ImmutableBytesPtr cacheKey = new ImmutableBytesPtr(lockKey); +PSchema schema = loadSchema(env, lockKey, cacheKey, clientTimeStamp, clientTimeStamp); +if (schema != null) { +if (schema.getTimeStamp() < clientTimeStamp) { + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_ALREADY_EXISTS); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} else { + builder.setReturnCode(MetaDataProtos.MutationCode.NEWER_SCHEMA_FOUND); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} +} +region.mutateRowsWithLocks(schemaMutations, Collections.<byte[]> emptySet(), HConstants.NO_NONCE, +HConstants.NO_NONCE); + +// Invalidate the cache - the next getTable call will add it +// TODO: consider loading the table that was just created here, +// patching up the parent table, and updating the cache +Cache<ImmutableBytesPtr, PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env) +.getMetaDataCache(); +if (cacheKey != null) { +metaDataCache.invalidate(cacheKey); +} + +// Get timeStamp from mutations - the above method sets it if +// it's unset +long currentTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_NOT_FOUND); +builder.setMutationTime(currentTimeStamp); +done.run(builder.build()); +return; +} finally { +region.releaseRowLocks(locks); +} +} catch (Throwable t) { +logger.error("createFunction failed", t); +ProtobufUtil.setControllerException(controller, ServerUtil.createIOException(schemaName, t)); +} +} + +@Override +public void dropSchema(RpcController controller, DropSchemaRequest request, RpcCallback done) { +String schemaName = null; +try { +List schemaMetaData = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +byte[] lockKey = SchemaUtil.getSchemaKey(schemaName); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region);
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58117768 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -3201,4 +3367,155 @@ private MetaDataMutationResult doDropFunction(long clientTimeStamp, List<byte[]> return new MetaDataMutationResult(MutationCode.FUNCTION_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); } + +@Override +public void createSchema(RpcController controller, CreateSchemaRequest request, +RpcCallback done) { +MetaDataResponse.Builder builder = MetaDataResponse.newBuilder(); +String schemaName = null; +try { +List schemaMutations = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(schemaMutations); + +byte[] lockKey = m.getRow(); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region); +if (result != null) { +done.run(MetaDataMutationResult.toProto(result)); +return; +} +List locks = Lists.newArrayList(); +long clientTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); +try { +acquireLock(region, lockKey, locks); +// Get as of latest timestamp so we can detect if we have a +// newer function that already +// exists without making an additional query +ImmutableBytesPtr cacheKey = new ImmutableBytesPtr(lockKey); +PSchema schema = loadSchema(env, lockKey, cacheKey, clientTimeStamp, clientTimeStamp); +if (schema != null) { +if (schema.getTimeStamp() < clientTimeStamp) { + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_ALREADY_EXISTS); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} else { + builder.setReturnCode(MetaDataProtos.MutationCode.NEWER_SCHEMA_FOUND); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} +} +region.mutateRowsWithLocks(schemaMutations, Collections.<byte[]> emptySet(), HConstants.NO_NONCE, +HConstants.NO_NONCE); + +// Invalidate the cache - the next getTable call will add it --- End diff -- Please update these comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58117686 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -3201,4 +3367,155 @@ private MetaDataMutationResult doDropFunction(long clientTimeStamp, List<byte[]> return new MetaDataMutationResult(MutationCode.FUNCTION_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); } + +@Override +public void createSchema(RpcController controller, CreateSchemaRequest request, +RpcCallback done) { +MetaDataResponse.Builder builder = MetaDataResponse.newBuilder(); +String schemaName = null; +try { +List schemaMutations = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(schemaMutations); + +byte[] lockKey = m.getRow(); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region); +if (result != null) { +done.run(MetaDataMutationResult.toProto(result)); +return; +} +List locks = Lists.newArrayList(); +long clientTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); +try { +acquireLock(region, lockKey, locks); +// Get as of latest timestamp so we can detect if we have a +// newer function that already --- End diff -- newer schema. Also, please format the comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58117457 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -2910,6 +3026,16 @@ private static MetaDataMutationResult checkTableKeyInRegion(byte[] key, Region r EnvironmentEdgeManager.currentTimeMillis(), null); } +private static MetaDataMutationResult checkSchemaKeyInRegion(byte[] key, Region region) { --- End diff -- You should reuse checkTableKeyInRegion and rename it to checkKeyInRegion. You would pass MutationCode to include in the result an extra parameter when the key is not in region. While you are it, also please remove the method checkFunctionKeyInRegion. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r58116856 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -3201,4 +3367,155 @@ private MetaDataMutationResult doDropFunction(long clientTimeStamp, List<byte[]> return new MetaDataMutationResult(MutationCode.FUNCTION_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); } + +@Override +public void createSchema(RpcController controller, CreateSchemaRequest request, +RpcCallback done) { +MetaDataResponse.Builder builder = MetaDataResponse.newBuilder(); +String schemaName = null; +try { +List schemaMutations = ProtobufUtil.getMutations(request); +schemaName = request.getSchemaName(); +Mutation m = MetaDataUtil.getPutOnlyTableHeaderRow(schemaMutations); + +byte[] lockKey = m.getRow(); +Region region = env.getRegion(); +MetaDataMutationResult result = checkSchemaKeyInRegion(lockKey, region); +if (result != null) { +done.run(MetaDataMutationResult.toProto(result)); +return; +} +List locks = Lists.newArrayList(); +long clientTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); +try { +acquireLock(region, lockKey, locks); +// Get as of latest timestamp so we can detect if we have a +// newer function that already +// exists without making an additional query +ImmutableBytesPtr cacheKey = new ImmutableBytesPtr(lockKey); +PSchema schema = loadSchema(env, lockKey, cacheKey, clientTimeStamp, clientTimeStamp); +if (schema != null) { +if (schema.getTimeStamp() < clientTimeStamp) { + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_ALREADY_EXISTS); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} else { + builder.setReturnCode(MetaDataProtos.MutationCode.NEWER_SCHEMA_FOUND); + builder.setMutationTime(EnvironmentEdgeManager.currentTimeMillis()); +builder.setSchema(PSchema.toProto(schema)); +done.run(builder.build()); +return; +} +} +region.mutateRowsWithLocks(schemaMutations, Collections.<byte[]> emptySet(), HConstants.NO_NONCE, +HConstants.NO_NONCE); + +// Invalidate the cache - the next getTable call will add it +// TODO: consider loading the table that was just created here, +// patching up the parent table, and updating the cache +Cache<ImmutableBytesPtr, PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env) +.getMetaDataCache(); +if (cacheKey != null) { +metaDataCache.invalidate(cacheKey); +} + +// Get timeStamp from mutations - the above method sets it if +// it's unset +long currentTimeStamp = MetaDataUtil.getClientTimeStamp(schemaMutations); + builder.setReturnCode(MetaDataProtos.MutationCode.SCHEMA_NOT_FOUND); +builder.setMutationTime(currentTimeStamp); +done.run(builder.build()); +return; +} finally { +region.releaseRowLocks(locks); +} +} catch (Throwable t) { +logger.error("createFunction failed", t); --- End diff -- Change the message to: "Creating the schema " + schemaName + " failed." --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57373598 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -958,6 +973,32 @@ private boolean allowOnlineTableSchemaUpdate() { QueryServicesOptions.DEFAULT_ALLOW_ONLINE_TABLE_SCHEMA_UPDATE); } +private NamespaceDescriptor ensureNamespaceCreated(String schemaName) throws SQLException { +SQLException sqlE = null; +try (HBaseAdmin admin = getAdmin()) { +final String quorum = ZKConfig.getZKQuorumServersString(config); +final String znode = this.props.get(HConstants.ZOOKEEPER_ZNODE_PARENT); +logger.debug("Found quorum: " + quorum + ":" + znode); +boolean nameSpaceExists = true; +NamespaceDescriptor namespaceDescriptor = null; +try { +namespaceDescriptor = admin.getNamespaceDescriptor(schemaName); --- End diff -- How about you try creating the name space and catch the expected NamespaceExistException? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57373401 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -958,6 +973,32 @@ private boolean allowOnlineTableSchemaUpdate() { QueryServicesOptions.DEFAULT_ALLOW_ONLINE_TABLE_SCHEMA_UPDATE); } +private NamespaceDescriptor ensureNamespaceCreated(String schemaName) throws SQLException { +SQLException sqlE = null; +try (HBaseAdmin admin = getAdmin()) { +final String quorum = ZKConfig.getZKQuorumServersString(config); --- End diff -- Remove this two variables and the following logger statement. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57373091 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java --- @@ -634,15 +642,19 @@ private PMetaData metaDataMutated(PName tenantId, String tableName, long tableSe } } -@Override -public PMetaData addColumn(final PName tenantId, final String tableName, final List columns, final long tableTimeStamp, -final long tableSeqNum, final boolean isImmutableRows, final boolean isWalDisabled, final boolean isMultitenant, -final boolean storeNulls, final boolean isTransactional, final long updateCacheFrequency, final long resolvedTime) throws SQLException { -return metaDataMutated(tenantId, tableName, tableSeqNum, new Mutator() { + @Override --- End diff -- Is this just white-space diff here? If yes, please revert the change. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57372836 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/PSchemaKey.java --- @@ -0,0 +1,67 @@ +/* --- End diff -- Where is this used? I don't see any mention of PSchemaKey outside this class itself. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57372367 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java --- @@ -3301,4 +3356,86 @@ private PTable getParentOfView(PTable view) throws SQLException { String parentName = SchemaUtil.normalizeFullTableName(select.getFrom().toString().trim()); return connection.getTable(new PTableKey(view.getTenantId(), parentName)); } + +public MutationState createSchema(CreateSchemaStatement create) throws SQLException { +boolean wasAutoCommit = connection.getAutoCommit(); +connection.rollback(); +try { +boolean isIfNotExists = create.isIfNotExists(); +PSchema schema = new PSchema(create.getSchemaName()); +connection.setAutoCommit(false); +List schemaMutations; + +try (PreparedStatement schemaUpsert = connection.prepareStatement(CREATE_SCHEMA)) { +schemaUpsert.setString(1, schema.getSchemaName()); +schemaUpsert.setString(2, MetaDataClient.EMPTY_TABLE); +schemaUpsert.execute(); +schemaMutations = connection.getMutationState().toMutations(null).next().getSecond(); +connection.rollback(); +} +MetaDataMutationResult result = connection.getQueryServices().createSchema(schemaMutations, +schema.getSchemaName()); +MutationCode code = result.getMutationCode(); +switch (code) { +case SCHEMA_ALREADY_EXISTS: +if (result.getTable() != null) { // Can happen for transactional table that already exists as HBase --- End diff -- result.getSchema() ? Also, please remove the comment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57371949 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -1115,6 +1212,24 @@ private PFunction loadFunction(RegionCoprocessorEnvironment env, byte[] key, return null; } +private PSchema loadSchema(RegionCoprocessorEnvironment env, byte[] key, ImmutableBytesPtr cacheKey, +long clientTimeStamp, long asOfTimeStamp) throws IOException, SQLException { +Region region = env.getRegion(); +Cache<ImmutableBytesPtr, PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env).getMetaDataCache(); +PSchema schema = (PSchema)metaDataCache.getIfPresent(cacheKey); +// We always cache the latest version - fault in if not in cache +if (schema != null) { return schema; } +ArrayList<byte[]> arrayList = new ArrayList<byte[]>(1); +arrayList.add(key); +List schemas = buildSchemas(arrayList, region, asOfTimeStamp, cacheKey); +if (schemas != null) return schemas.get(0); +// if not found then check if newer table already exists and add delete marker for timestamp --- End diff -- Change comment to mention schema and not table. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57371653 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java --- @@ -544,7 +556,28 @@ private MetaDataMutationResult updateCache(PName tenantId, String schemaName, St return result; } - + +public MetaDataMutationResult updateCache(String schemaName) throws SQLException { +return updateCache(schemaName, false); +} + +public MetaDataMutationResult updateCache(String schemaName, boolean alwaysHitServer) throws SQLException { +long clientTimeStamp = getClientTimeStamp(); +PSchema schema = null; +try { +schema = connection.getMetaDataCache().getSchema(new PTableKey(null, schemaName)); --- End diff -- Is tenantId a factor here? Is it always ok to have tenantId as null. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] phoenix pull request: PHOENIX-1311 HBase namespaces surfaced in ph...
Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/153#discussion_r57371542 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/parse/PSchema.java --- @@ -0,0 +1,86 @@ +/* + * 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.parse; + +import org.apache.hadoop.hbase.HConstants; +import org.apache.phoenix.coprocessor.generated.PSchemaProtos; +import org.apache.phoenix.schema.PMetaDataEntity; +import org.apache.phoenix.schema.PName; +import org.apache.phoenix.schema.PNameFactory; +import org.apache.phoenix.schema.PTableKey; +import org.apache.phoenix.util.SchemaUtil; +import org.apache.phoenix.util.SizedUtil; + +public class PSchema implements PMetaDataEntity { + +private final PName schemaName; +private PTableKey schemaKey; +private long timeStamp; +private int estimatedSize; + +public PSchema(long timeStamp) { // For index delete marker +this.timeStamp = timeStamp; +this.schemaName = null; +} + +public PSchema(String schemaName) { +this(schemaName, HConstants.LATEST_TIMESTAMP); +} + +public PSchema(String schemaName, long timeStamp) { +this.schemaName = PNameFactory.newName(SchemaUtil.normalizeIdentifier(schemaName)); +this.schemaKey = new PTableKey(null, this.schemaName.getString()); --- End diff -- Thinking about this a little bit more, is it possible for tenant views on the same multi-tenant table to have different schema/namespaces? If yes, how would you differentiate these schemas considering the tenantId is being set as null here. At a minimum, it would be ideal to have some testing around this scenario. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---