virajjasani commented on pull request #920:
URL: https://github.com/apache/phoenix/pull/920#issuecomment-710038097
@ChinmaySKulkarni IMHO we can not live without `tableExists()` call because
`Connection.getTable()` has no guarantee that it will throw
`TableNotFoundException` if table doesn't exist. What happens during namespace
upgrade is that even if `SYSTEM.MUTEX` has been deleted (removed entry from
meta), `Connection.getTable()` still returns us an instance of `Table`,
however, we can't perform actions on it.
Admin.tableExists is the only thread safe way to identify if table already
exists:
```
/**
* @param tableName Table to check.
* @return <code>true</code> if table exists already.
* @throws IOException if a remote or network exception occurs
*/
boolean tableExists(TableName tableName) throws IOException;
```
&
```
/**
* Retrieve a Table implementation for accessing a table.
* The returned Table is not thread safe, a new instance should be created
for each using thread.
* This is a lightweight operation, pooling or caching of the returned
Table
* is neither required nor desired.
* <p>
* The caller is responsible for calling {@link Table#close()} on the
returned
* table instance.
* <p>
* Since 0.98.1 this method no longer checks table existence. An exception
* will be thrown if the table does not exist only when the first
operation is
* attempted.
* @param tableName the name of the table
* @return a Table to use for interactions with this table
*/
default Table getTable(TableName tableName) throws IOException {
return getTable(tableName, null);
}
```
I checked implementation also, and getTable() internally uses
`getTableBuilder()` which just provides a builder instance for just creating
`Table` object to access actual table, but there is no guarantee that a table
exists while accessing it. Only existsTable() goes to meta table and checks for
the existing entry of Table.
We can do one improvement though, we can utilize same connection to perform
both: tableExists() and getTable().
If we don't have `tableExists()`, test cases
`MigrateSystemTablesToSystemNamespaceIT` are guaranteed to fail.
Updating the PR accordingly.
----------------------------------------------------------------
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:
[email protected]
