sanjeet006py commented on code in PR #2594:
URL: https://github.com/apache/phoenix/pull/2594#discussion_r3864290226


##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -2459,6 +2460,53 @@ private void disableTable(Admin admin, TableName 
tableName) throws IOException {
     }
   }
 
+  private void enableTable(Admin admin, TableName tableName) throws 
IOException {
+    try {
+      admin.enableTable(tableName);
+    } catch (TableNotDisabledException e) {
+      LOGGER.info("Table already enabled, continuing with next steps", e);
+    }
+  }
+
+  /**
+   * PHOENIX-7788: re-enable a disabled physical HBase table if SYSTEM.CATALOG 
has no row for it. If
+   * metadata exists, leave it disabled — an admin may have disabled the 
registered table.
+   */
+  private void reenableOrphanedDisabledHBaseTable(byte[] schemaBytes, byte[] 
tableBytes,
+    boolean isNamespaceMapped, PTableType tableType) throws SQLException {
+    if (tableType != PTableType.TABLE) {
+      return;
+    }
+    TableName physicalTableName = TableName.valueOf(
+      SchemaUtil.getPhysicalHBaseTableName(schemaBytes, tableBytes, 
isNamespaceMapped).getBytes());
+    try (Admin admin = getAdmin()) {
+      if (!AdminUtilWithFallback.tableExists(admin, physicalTableName)) {
+        return;
+      }
+      if (!admin.isTableDisabled(physicalTableName)) {
+        return;
+      }
+      MetaDataMutationResult result = getTable(null, schemaBytes, tableBytes,
+        HConstants.LATEST_TIMESTAMP, HConstants.LATEST_TIMESTAMP);
+      if (result.getMutationCode() != MutationCode.TABLE_NOT_FOUND) {
+        LOGGER.info(
+          "Physical HBase table {} is disabled but SYSTEM.CATALOG has metadata 
for it "

Review Comment:
   nit: Instead of hardcoding `SYSTEM.CATALOG` how about using 
`PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME`? This will keep code DRY.



##########
phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -2557,6 +2605,9 @@ public MetaDataMutationResult createTable(final 
List<Mutation> tableMetaData,
       (tableType != PTableType.CDC) && ((tableType == PTableType.VIEW && 
physicalTableName != null)
         || (tableType != PTableType.VIEW && (physicalTableName == null || 
localIndexTable)))
     ) {
+      // PHOENIX-7788: recover from an orphaned disabled physical table before 
ensureTableCreated
+      // runs modifyTable on it. See the helper for the metadata-preserving 
contract.
+      reenableOrphanedDisabledHBaseTable(schemaBytes, tableBytes, 
isNamespaceMapped, tableType);

Review Comment:
   How about shifting this to `ensureTableCreated`? That way the responsibility 
of ensuring table is created and usable lies with `ensureTableCreated` only. 
Further, you will notice that in subsequent lines `ensureViewIndexTableCreated` 
is called which eventually calls `ensureTableCreated` for physical HBase table 
used by view indexes. And, same situation of physical HBase table for view 
index being disabled can happen here also. 



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to