virajjasani commented on a change in pull request #920: URL: https://github.com/apache/phoenix/pull/920#discussion_r509509727
########## File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ########## @@ -4364,17 +4353,17 @@ public void deleteMutexCell(String tenantId, String schemaName, String tableName } } - private byte[] getSysMutexPhysicalTableNameBytes() throws IOException, SQLException { - byte[] sysMutexPhysicalTableNameBytes = null; - try(Admin admin = getAdmin()) { - if(admin.tableExists(PhoenixDatabaseMetaData.SYSTEM_MUTEX_HBASE_TABLE_NAME)) { - sysMutexPhysicalTableNameBytes = PhoenixDatabaseMetaData.SYSTEM_MUTEX_NAME_BYTES; - } else if (admin.tableExists(TableName.valueOf( - SchemaUtil.getPhysicalTableName(SYSTEM_MUTEX_NAME, props).getName()))) { - sysMutexPhysicalTableNameBytes = SchemaUtil.getPhysicalTableName(SYSTEM_MUTEX_NAME, props).getName(); + private Table getSysMutexTable() throws SQLException, IOException { + String table = SYSTEM_MUTEX_NAME; + TableName tableName = TableName.valueOf(table); + try (Admin admin = getAdmin()) { + if (!admin.tableExists(tableName)) { + table = table.replace(QueryConstants.NAME_SEPARATOR, + QueryConstants.NAMESPACE_SEPARATOR); + tableName = TableName.valueOf(table); } + return connection.getTable(tableName); Review comment: > This scenario is the same as returning null from the previous getSysMutexPhysicalTableNameBytes() method and then attempting to retrieve a null table. Basically, we are certain that either `SYSTEM.MUTEX` or `SYSTEM:MUTEX` exist at any point in time right? e.g if we are performing namespace upgrade (e.g MigrateSystemTablesToSystemNamespaceIT test), when SYSTEM.MUTEX no longer exists, if we call `HBaseFactoryProvider.getHTableFactory().getTable()`, it does return `Table` object, it doesn't return null as such. However, using that table object, we can't perform any operation because actual table would not exist. As for this comment, the scenario is not same as previous `getSysMutexPhysicalTableNameBytes()` because when we reach at this point, based on above `Admin.tableExists()` call, we would call `connection.getTable()` with correct table name only (I hope we never have situation where SYSTEM.MUTEX and SYSTEM:MUTEX both are dropped. Should we really worry about this case?) ---------------------------------------------------------------- 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