[
https://issues.apache.org/jira/browse/PHOENIX-6203?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17228290#comment-17228290
]
ASF GitHub Bot commented on PHOENIX-6203:
-----------------------------------------
yanxinyi commented on a change in pull request #957:
URL: https://github.com/apache/phoenix/pull/957#discussion_r519481052
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
##########
@@ -1440,5 +1441,69 @@ public void
testAddNonPKColumnWhenlastPKIsVARBINARYOrARRAY() throws Exception {
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(e.getTableName(), tableName);
+ }
+
+ String tableName1 = SchemaUtil.getTableName(schemaName,
dataTableName);
+ String ddl = "CREATE TABLE " + tableName1
+ + " (col1 INTEGER PRIMARY KEY, col2 INTEGER)";
+ conn.createStatement().execute(ddl);
+ String tableName2 = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ ddl = "CREATE TABLE " + tableName2
+ + " (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(tableName1));
+ assertNotNull(table1);
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(tableName1));
+ } 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(tableName1));
+ assertEquals(t1.getName().getNameAsString(), tableName1);
+ Table t2 = cqs.getTable(Bytes.toBytes(tableName2));
+ assertEquals(t2.getName().getNameAsString(), tableName2);
+
+ // this is correct check for existence of table
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(tableName1));
+ fail("Should have thrown TableNotFoundException");
+ } catch (TableNotFoundException e) {
+ // exact match is possible because schema name is empty
+ assertEquals(e.getTableName(), tableName1);
+ }
+ try {
+ cqs.getTableIfExists(Bytes.toBytes(tableName2));
+ fail("Should have thrown TableNotFoundException");
+ } catch (TableNotFoundException e) {
+ // just asserting table name, not schema name
+ assertTrue(tableName2.contains(e.getTableName()));
Review comment:
nit: can we use the same way as above, which is assertEquals?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> CQS.getTable(byte[] tableName) does not throw TNFE even if table doesn't exist
> ------------------------------------------------------------------------------
>
> Key: PHOENIX-6203
> URL: https://issues.apache.org/jira/browse/PHOENIX-6203
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 5.0.0, 4.15.0
> Reporter: Viraj Jasani
> Assignee: Viraj Jasani
> Priority: Major
> Fix For: 5.1.0, 4.16.0
>
>
> CQSI.getTable(byte[] tableName)
> ([here|https://github.com/apache/phoenix/blob/master/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java#L481])
> is being used at multiple places to retrieve Table object from Connection
> which is opened by CQSI.
> However, getTable() seems to be throwing TNFE with the expectation that
> underlying Connection.getTable(TableName tableName) will throw
> org.apache.hadoop.hbase.TNFE , which never happens because
> Connection.getTable() only returns Table implementation to access table,
> however, it does not take into consideration whether given table exists.
> Admin.tableExists() is the only reliable API to determine whether given table
> exists.
> One simple test to ensure the above finding holds true with current
> CQSI.getTable() method.
> {code:java}
> @Test
> public void test1() throws Exception {
> try (Connection conn = DriverManager.getConnection(getJdbcUrl())) {
> String tableName = SchemaUtil.getTableName(generateUniqueName(),
> generateUniqueName());
> // create parent table
> String ddl = "CREATE TABLE " + tableName
> + " (col1 INTEGER NOT NULL, col2 INTEGER " + "CONSTRAINT pk PRIMARY
> KEY (col1))";
> conn.createStatement().execute(ddl);
> ConnectionQueryServices
> cqs=conn.unwrap(PhoenixConnection.class).getQueryServices();
> // table does exist and cqs.getTable() does not throw TNFE
> Table table1 = cqs.getTable(Bytes.toBytes(tableName));
> assertNotNull(table1);
> // this is correct check for existence of table
> assertTrue(cqs.getAdmin().tableExists(TableName.valueOf(tableName)));
> conn.createStatement().execute("DROP TABLE " + tableName);
> // table has been dropped i.e does not exist, still cqs.getTable()
> does not throw TNFE
> Table table2 = cqs.getTable(Bytes.toBytes(tableName));
> assertNotNull(table2);
> assertEquals(table1.getName(), table2.getName());
> // this is correct check for existence of table
> assertFalse(cqs.getAdmin().tableExists(TableName.valueOf(tableName)));
> // table never existed, still cqs.getTable() does not throw TNFE
> Table table3 = cqs.getTable(Bytes.toBytes("abc"));
> assertNotNull(table3);
> assertEquals(table3.getName(), TableName.valueOf("abc"));
> // this is correct check for existence of table
> assertFalse(cqs.getAdmin().tableExists(TableName.valueOf("abc")));
> }
> }
> {code}
>
> Since CQS.getTable() is basic utility, it is being used at many places. We
> might have to prefer one of these solutions:
> # We change CQSI.getTable() to actually use Admin.tableExists() and
> accordingly either return Table object from Connection or throw TNFE
> # Identify all callers of CQSI.getTable() and determine how many of them
> actually needs to know whether table exist and accordingly use
> CQSI.getTable(), (maybe by passing a flag) and catch TNFE in all such callers
> (which they might already be doing with expectation of catching TNFE cases).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
