[GitHub] phoenix pull request #232: Pull request for merging the encode columns branc...

2017-03-15 Thread samarthjain
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...

2017-02-23 Thread samarthjain
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....

2017-02-14 Thread samarthjain
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...

2017-02-14 Thread samarthjain
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 #:

2016-12-22 Thread samarthjain
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...

2016-08-15 Thread samarthjain
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...

2016-08-15 Thread samarthjain
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...

2016-08-15 Thread samarthjain
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...

2016-08-15 Thread samarthjain
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...

2016-08-15 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-12 Thread samarthjain
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...

2016-08-11 Thread samarthjain
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...

2016-08-11 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-10 Thread samarthjain
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...

2016-08-08 Thread samarthjain
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...

2016-08-08 Thread samarthjain
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...

2016-08-08 Thread samarthjain
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...

2016-04-13 Thread samarthjain
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...

2016-04-13 Thread samarthjain
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...

2016-04-13 Thread samarthjain
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...

2016-04-13 Thread samarthjain
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...

2016-04-12 Thread samarthjain
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...

2016-04-11 Thread samarthjain
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...

2016-04-11 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-08 Thread samarthjain
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...

2016-04-07 Thread samarthjain
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...

2016-04-07 Thread samarthjain
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...

2016-04-07 Thread samarthjain
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...

2016-04-07 Thread samarthjain
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...

2016-04-05 Thread samarthjain
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...

2016-04-05 Thread samarthjain
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...

2016-04-05 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-04-04 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-31 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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...

2016-03-24 Thread samarthjain
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.
---


  1   2   >