[PHOENIX-6203] CQS.getTable(byte[] tableName) does not throw TNFE even if table doesn't exist

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.0.0, 4.15.0
• Fix Version/s: 5.1.0, 4.16.0
• Assignee: Viraj Jasani
• Reporter: Viraj Jasani
• Created: 21/Oct/20 19:01
• Updated: 09/Nov/20 19:23
• Resolved: 09/Nov/20 18:12

Description

CQSI.getTable(byte[] tableName) (here) 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.

@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")));
    }
}

 

Since CQS.getTable() is basic utility, it is being used at many places. We might have to prefer one of these solutions:

1. We change CQSI.getTable() to actually use Admin.tableExists() and accordingly either return Table object from Connection or throw TNFE
2. 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).

Issue Links

• links to: GitHub Pull Request #957