This is an automated email from the ASF dual-hosted git repository.
yanxinyi pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new f0f2f74 PHOENIX-6203 : Add new CQS method to return Table instance
only if table exists
f0f2f74 is described below
commit f0f2f7486911172670b415544cb654bd696590d7
Author: Viraj Jasani <vjas...@apache.org>
AuthorDate: Mon Nov 9 14:06:32 2020 +0530
PHOENIX-6203 : Add new CQS method to return Table instance only if table
exists
Signed-off-by: Xinyi Yan <yanxi...@apache.org>
---
.../org/apache/phoenix/end2end/AlterTableIT.java | 74 +++++++++++++++++++++-
.../phoenix/query/ConnectionQueryServices.java | 17 +++++
.../phoenix/query/ConnectionQueryServicesImpl.java | 44 +++++++++----
.../query/ConnectionlessQueryServicesImpl.java | 6 ++
.../query/DelegateConnectionQueryServices.java | 6 ++
.../java/org/apache/phoenix/query/BaseTest.java | 4 +-
.../query/ConnectionQueryServicesImplTest.java | 15 ++---
7 files changed, 142 insertions(+), 24 deletions(-)
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
index b60f581..3dafc59 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
@@ -48,12 +48,15 @@ import java.util.Properties;
import org.apache.hadoop.hbase.HColumnDescriptor;
import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.HBaseAdmin;
+import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.query.BaseTest;
+import org.apache.phoenix.query.ConnectionQueryServices;
import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.schema.PTable;
@@ -1440,5 +1443,74 @@ public class AlterTableIT extends
ParallelStatsDisabledIT {
stmt.execute(alterDdl2);
}
}
+
+ @Test
+ public void testTableExists() throws Exception {
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ ConnectionQueryServices cqs =
+ conn.unwrap(PhoenixConnection.class).getQueryServices();
+ String tableName = "randomTable";
+ // table never existed, still cqs.getTable() does not throw TNFE
+ Table randomTable = cqs.getTable(Bytes.toBytes(tableName));
+ assertNotNull(randomTable);
+ assertEquals(randomTable.getName(), TableName.valueOf(tableName));
+ try {
+ // this is correct check for existence of table
+ cqs.getTableIfExists(Bytes.toBytes(tableName));
+ fail("Should have thrown TableNotFoundException");
+ } catch (TableNotFoundException e) {
+ assertEquals(tableName, e.getTableName());
+ }
+
+ String fullTableName1 = SchemaUtil.getTableName(schemaName,
+ dataTableName);
+ String ddl = "CREATE TABLE " + fullTableName1
+ + " (col1 INTEGER PRIMARY KEY, col2 INTEGER)";
+ conn.createStatement().execute(ddl);
+ String schemaName2 = generateUniqueName();
+ String tableName2 = generateUniqueName();
+ String fullTableName2 = SchemaUtil.getTableName(schemaName2,
+ tableName2);
+ ddl = "CREATE TABLE " + fullTableName2
+ + " (col1 INTEGER PRIMARY KEY, col2 INTEGER)";
+ conn.createStatement().execute(ddl);
+
+ // table does exist and cqs.getTable() does not throw TNFE
+ Table table1 = cqs.getTable(Bytes.toBytes(fullTableName1));
+ assertNotNull(table1);
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(fullTableName1));
+ } catch (TableNotFoundException e) {
+ fail("Should not throw TableNotFoundException");
+ }
+
+ disableAndDropNonSystemTables();
+ // tables have been dropped, still cqs.getTable()
+ // does not throw TNFE for tableName1 and tableName2
+ Table t1 = cqs.getTable(Bytes.toBytes(fullTableName1));
+ assertEquals(t1.getName().getNameAsString(), fullTableName1);
+ Table t2 = cqs.getTable(Bytes.toBytes(fullTableName2));
+ assertEquals(t2.getName().getNameAsString(), fullTableName2);
+
+ // this is correct check for existence of table
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(fullTableName1));
+ fail("Should have thrown TableNotFoundException");
+ } catch (TableNotFoundException e) {
+ // match table and schema
+ assertEquals(dataTableName, e.getTableName());
+ assertEquals(schemaName, e.getSchemaName());
+ }
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(fullTableName2));
+ fail("Should have thrown TableNotFoundException");
+ } catch (TableNotFoundException e) {
+ // match table and schema
+ assertEquals(tableName2, e.getTableName());
+ assertEquals(schemaName2, e.getSchemaName());
+ }
+
+ }
+ }
+
}
-
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
index db63d48..40bbff3 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServices.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.hbase.HTableDescriptor;
import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.HTableInterface;
import org.apache.hadoop.hbase.client.Mutation;
+import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.util.Pair;
@@ -72,6 +73,22 @@ public interface ConnectionQueryServices extends
QueryServices, MetaDataMutated
*/
public HTableInterface getTable(byte[] tableName) throws SQLException;
+ /**
+ * Get Table by the given name. It is the responsibility of callers
+ * to close the returned Table interface. This method uses additional Admin
+ * API to ensure if table exists before returning Table interface from
+ * Connection. If table does not exist, this method will throw
+ * {@link org.apache.phoenix.schema.TableNotFoundException}
+ *
+ * @param tableName the name of the Table
+ * @return Table interface
+ * @throws SQLException If something goes wrong while retrieving table
+ * interface from connection managed by implementor. If table does not
+ * exist, {@link org.apache.phoenix.schema.TableNotFoundException} will
+ * be thrown.
+ */
+ Table getTableIfExists(byte[] tableName) throws SQLException;
+
public HTableDescriptor getTableDescriptor(byte[] tableName) throws
SQLException;
public HRegionLocation getTableRegionLocation(byte[] tableName, byte[]
row) throws SQLException;
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index a0512bd..55bb9fe 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -127,6 +127,7 @@ import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.hadoop.hbase.NamespaceNotFoundException;
import org.apache.hadoop.hbase.TableExistsException;
import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.HBaseAdmin;
@@ -486,15 +487,28 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
@Override
public HTableInterface getTable(byte[] tableName) throws SQLException {
try {
- return HBaseFactoryProvider.getHTableFactory().getTable(tableName,
connection, null);
- } catch (org.apache.hadoop.hbase.TableNotFoundException e) {
- throw new
TableNotFoundException(SchemaUtil.getSchemaNameFromFullName(tableName),
SchemaUtil.getTableNameFromFullName(tableName));
+ return HBaseFactoryProvider.getHTableFactory().getTable(tableName,
+ connection, null);
} catch (IOException e) {
throw new SQLException(e);
}
}
@Override
+ public Table getTableIfExists(byte[] tableName) throws SQLException {
+ try (Admin admin = getAdmin()) {
+ if (!admin.tableExists(TableName.valueOf(tableName))) {
+ throw new TableNotFoundException(
+ SchemaUtil.getSchemaNameFromFullName(tableName),
+ SchemaUtil.getTableNameFromFullName(tableName));
+ }
+ } catch (IOException e) {
+ throw new SQLException(e);
+ }
+ return getTable(tableName);
+ }
+
+ @Override
public HTableDescriptor getTableDescriptor(byte[] tableName) throws
SQLException {
HTableInterface htable = getTable(tableName);
try {
@@ -4498,17 +4512,21 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
@VisibleForTesting
- public Table getSysMutexTable() throws SQLException, IOException {
- String table = SYSTEM_MUTEX_NAME;
- TableName tableName = TableName.valueOf(table);
- try (HBaseAdmin admin = getAdmin()) {
- if (!admin.tableExists(tableName)) {
- table = table.replace(QueryConstants.NAME_SEPARATOR,
- QueryConstants.NAMESPACE_SEPARATOR);
- tableName = TableName.valueOf(table);
- }
- return connection.getTable(tableName);
+ public Table getSysMutexTable() throws SQLException {
+ String tableNameAsString = SYSTEM_MUTEX_NAME;
+ Table table;
+ try {
+ table = getTableIfExists(Bytes.toBytes(tableNameAsString));
+ } catch (TableNotFoundException e) {
+ tableNameAsString = tableNameAsString.replace(
+ QueryConstants.NAME_SEPARATOR,
+ QueryConstants.NAMESPACE_SEPARATOR);
+ // if SYSTEM.MUTEX does not exist, we don't need to check
+ // for the existence of SYSTEM:MUTEX as it must exist, hence
+ // we can call getTable() here instead of getTableIfExists()
+ table = getTable(Bytes.toBytes(tableNameAsString));
}
+ return table;
}
private String addColumn(String columnsToAddSoFar, String columns) {
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
index bc636ab..9679734 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java
@@ -41,6 +41,7 @@ import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.HTableInterface;
import org.apache.hadoop.hbase.client.Mutation;
+import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.util.Addressing;
@@ -212,6 +213,11 @@ public class ConnectionlessQueryServicesImpl extends
DelegateQueryServices imple
}
@Override
+ public Table getTableIfExists(byte[] tableName) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws
SQLException {
List<HRegionLocation> regions =
tableSplits.get(Bytes.toString(tableName));
if (regions != null) {
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
index 8f3273e..d271547 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.hbase.HTableDescriptor;
import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.HTableInterface;
import org.apache.hadoop.hbase.client.Mutation;
+import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.util.Pair;
@@ -75,6 +76,11 @@ public class DelegateConnectionQueryServices extends
DelegateQueryServices imple
}
@Override
+ public Table getTableIfExists(byte[] tableName) throws SQLException {
+ return getDelegate().getTableIfExists(tableName);
+ }
+
+ @Override
public List<HRegionLocation> getAllTableRegions(byte[] tableName) throws
SQLException {
return getDelegate().getAllTableRegions(tableName);
}
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
index a1e794a..fc9df19 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
@@ -1555,9 +1555,9 @@ public abstract class BaseTest {
}
/**
- * Disable and drop all the tables except SYSTEM.CATALOG and
SYSTEM.SEQUENCE
+ * Disable and drop all non system tables
*/
- private static void disableAndDropNonSystemTables() throws Exception {
+ protected static void disableAndDropNonSystemTables() throws Exception {
if (driver == null) return;
HBaseAdmin admin = driver.getConnectionQueryServices(null,
null).getAdmin();
try {
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
index da16dfe..6cf0140 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
@@ -58,6 +58,7 @@ import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.util.ReadOnlyProps;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
@@ -112,6 +113,8 @@ public class ConnectionQueryServicesImplTest {
when(mockCqs.updateAndConfirmSplitPolicyForTask(SYS_TASK_TDB_SP))
.thenCallRealMethod();
when(mockCqs.getSysMutexTable()).thenCallRealMethod();
+ when(mockCqs.getTable(Matchers.<byte[]>any())).thenCallRealMethod();
+
when(mockCqs.getTableIfExists(Matchers.<byte[]>any())).thenCallRealMethod();
}
@SuppressWarnings("unchecked")
@@ -210,26 +213,22 @@ public class ConnectionQueryServicesImplTest {
@Test
public void testGetSysMutexTableWithName() throws Exception {
when(mockAdmin.tableExists(any(TableName.class))).thenReturn(true);
- when(mockConn.getTable(TableName.valueOf("SYSTEM.MUTEX")))
- .thenReturn(mockTable);
when(mockCqs.getAdmin()).thenReturn(mockAdmin);
+
when(mockCqs.getTable(Bytes.toBytes("SYSTEM.MUTEX"))).thenReturn(mockTable);
assertSame(mockCqs.getSysMutexTable(), mockTable);
verify(mockAdmin, Mockito.times(1)).tableExists(any(TableName.class));
- verify(mockConn, Mockito.times(1))
- .getTable(TableName.valueOf("SYSTEM.MUTEX"));
verify(mockCqs, Mockito.times(1)).getAdmin();
+ verify(mockCqs, Mockito.times(2)).getTable(Matchers.<byte[]>any());
}
@Test
public void testGetSysMutexTableWithNamespace() throws Exception {
when(mockAdmin.tableExists(any(TableName.class))).thenReturn(false);
- when(mockConn.getTable(TableName.valueOf("SYSTEM:MUTEX")))
- .thenReturn(mockTable);
when(mockCqs.getAdmin()).thenReturn(mockAdmin);
+
when(mockCqs.getTable(Bytes.toBytes("SYSTEM:MUTEX"))).thenReturn(mockTable);
assertSame(mockCqs.getSysMutexTable(), mockTable);
verify(mockAdmin, Mockito.times(1)).tableExists(any(TableName.class));
- verify(mockConn, Mockito.times(1))
- .getTable(TableName.valueOf("SYSTEM:MUTEX"));
verify(mockCqs, Mockito.times(1)).getAdmin();
+ verify(mockCqs, Mockito.times(2)).getTable(Matchers.<byte[]>any());
}
}
