[ https://issues.apache.org/jira/browse/PHOENIX-6203?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17228774#comment-17228774 ]
ASF GitHub Bot commented on PHOENIX-6203: ----------------------------------------- virajjasani commented on pull request #957: URL: https://github.com/apache/phoenix/pull/957#issuecomment-724225188 Thanks for the review @yanxinyi ---------------------------------------------------------------- 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 > > Attachments: PHOENIX-6203.4.x.000.patch, PHOENIX-6203.master.000.patch > > > 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)