Ignore unknown table formats when querying system.jdbc.columns table

imjalpreet · imjalpreet · commit 05a28cea0009 · 2025-08-14T21:38:34.000+05:30
diff --git a/presto-hive-common/src/main/java/com/facebook/presto/hive/HiveErrorCode.java b/presto-hive-common/src/main/java/com/facebook/presto/hive/HiveErrorCode.java
@@ -76,6 +76,7 @@ public enum HiveErrorCode
     HIVE_RANGER_SERVER_ERROR(48, EXTERNAL),
     HIVE_FUNCTION_INITIALIZATION_ERROR(49, EXTERNAL),
     HIVE_METASTORE_INITIALIZE_SSL_ERROR(50, EXTERNAL),
+    UNKNOWN_TABLE_TYPE(51, EXTERNAL),
     /**/;
 
     private final ErrorCode errorCode;
diff --git a/presto-hive-common/src/main/java/com/facebook/presto/hive/UnknownTableTypeException.java b/presto-hive-common/src/main/java/com/facebook/presto/hive/UnknownTableTypeException.java
@@ -11,18 +11,17 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.facebook.presto.iceberg;
+package com.facebook.presto.hive;
 
 import com.facebook.presto.spi.PrestoException;
-import com.facebook.presto.spi.SchemaTableName;
 
-import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_UNKNOWN_TABLE_TYPE;
+import static com.facebook.presto.hive.HiveErrorCode.UNKNOWN_TABLE_TYPE;
 
 public class UnknownTableTypeException
         extends PrestoException
 {
-    public UnknownTableTypeException(SchemaTableName tableName)
+    public UnknownTableTypeException(String message)
     {
-        super(ICEBERG_UNKNOWN_TABLE_TYPE, "Not an Iceberg table: " + tableName);
+        super(UNKNOWN_TABLE_TYPE, message);
     }
 }
diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java b/presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java
@@ -15,6 +15,7 @@
 
 import com.facebook.airlift.json.JsonCodec;
 import com.facebook.airlift.json.smile.SmileCodec;
+import com.facebook.airlift.log.Logger;
 import com.facebook.presto.common.CatalogSchemaName;
 import com.facebook.presto.common.Page;
 import com.facebook.presto.common.Subfield;
@@ -389,6 +390,7 @@
 public class HiveMetadata
         implements TransactionalMetadata
 {
+    private static final Logger log = Logger.get(HiveMetadata.class);
     public static final Set<String> RESERVED_ROLES = ImmutableSet.of("all", "default", "none");
     public static final String REFERENCED_MATERIALIZED_VIEWS = "referenced_materialized_views";
 
@@ -674,7 +676,7 @@ private ConnectorTableMetadata getTableMetadata(Optional<Table> table, SchemaTab
         }
 
         if (isIcebergTable(table.get()) || isDeltaLakeTable(table.get())) {
-            throw new PrestoException(HIVE_UNSUPPORTED_FORMAT, format("Not a Hive table '%s'", tableName));
+            throw new UnknownTableTypeException("Not a Hive table: " + tableName);
         }
 
         List<TableConstraint<String>> tableConstraints = metastore.getTableConstraints(metastoreContext, tableName.getSchemaName(), tableName.getTableName());
@@ -862,6 +864,9 @@ public Map<SchemaTableName, List<ColumnMetadata>> listTableColumns(ConnectorSess
             catch (TableNotFoundException e) {
                 // table disappeared during listing operation
             }
+            catch (UnknownTableTypeException e) {
+                log.warn(String.format("%s: Unknown table type of table %s", e.getMessage(), tableName));
+            }
         }
         return columns.build();
     }
diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/HiveTableOperations.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/HiveTableOperations.java
@@ -17,6 +17,7 @@
 import com.facebook.airlift.log.Logger;
 import com.facebook.presto.hive.HdfsContext;
 import com.facebook.presto.hive.HdfsEnvironment;
+import com.facebook.presto.hive.UnknownTableTypeException;
 import com.facebook.presto.hive.metastore.ExtendedHiveMetastore;
 import com.facebook.presto.hive.metastore.HivePrivilegeInfo;
 import com.facebook.presto.hive.metastore.MetastoreContext;
@@ -215,7 +216,7 @@ public TableMetadata refresh()
         Table table = getTable();
 
         if (!isIcebergTable(table)) {
-            throw new UnknownTableTypeException(getSchemaTableName());
+            throw new UnknownTableTypeException("Not an Iceberg table: " + getSchemaTableName());
         }
 
         if (isPrestoView(table)) {
diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java
@@ -28,6 +28,7 @@
 import com.facebook.presto.hive.HiveOutputMetadata;
 import com.facebook.presto.hive.HivePartition;
 import com.facebook.presto.hive.NodeVersion;
+import com.facebook.presto.hive.UnknownTableTypeException;
 import com.facebook.presto.iceberg.changelog.ChangelogOperation;
 import com.facebook.presto.iceberg.changelog.ChangelogUtil;
 import com.facebook.presto.iceberg.statistics.StatisticsFileCache;
diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java
@@ -24,7 +24,6 @@
 public enum IcebergErrorCode
         implements ErrorCodeSupplier
 {
-    ICEBERG_UNKNOWN_TABLE_TYPE(0, EXTERNAL),
     ICEBERG_INVALID_METADATA(1, EXTERNAL),
     ICEBERG_TOO_MANY_OPEN_PARTITIONS(2, USER_ERROR),
     ICEBERG_INVALID_PARTITION_VALUE(3, EXTERNAL),
diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergHiveMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergHiveMetadata.java
@@ -24,6 +24,7 @@
 import com.facebook.presto.hive.HiveTypeTranslator;
 import com.facebook.presto.hive.NodeVersion;
 import com.facebook.presto.hive.TableAlreadyExistsException;
+import com.facebook.presto.hive.UnknownTableTypeException;
 import com.facebook.presto.hive.ViewAlreadyExistsException;
 import com.facebook.presto.hive.metastore.Column;
 import com.facebook.presto.hive.metastore.Database;
@@ -230,7 +231,7 @@ protected boolean tableExists(ConnectorSession session, SchemaTableName schemaTa
             return false;
         }
         if (!isIcebergTable(hiveTable.get())) {
-            throw new UnknownTableTypeException(schemaTableName);
+            throw new UnknownTableTypeException("Not an Iceberg table: " + schemaTableName);
         }
         return true;
     }
diff --git a/presto-product-tests/src/main/java/com/facebook/presto/tests/SystemConnectorTests.java b/presto-product-tests/src/main/java/com/facebook/presto/tests/SystemConnectorTests.java
@@ -13,14 +13,20 @@
  */
 package com.facebook.presto.tests;
 
+import com.google.common.collect.ImmutableList;
 import io.prestodb.tempto.ProductTest;
+import io.prestodb.tempto.assertions.QueryAssert;
 import org.testng.annotations.Test;
 
 import java.sql.JDBCType;
+import java.util.List;
+import java.util.stream.Collectors;
 
 import static com.facebook.presto.tests.TestGroups.JDBC;
 import static com.facebook.presto.tests.TestGroups.SYSTEM_CONNECTOR;
 import static com.facebook.presto.tests.utils.JdbcDriverUtils.usingTeradataJdbcDriver;
+import static com.facebook.presto.tests.utils.QueryExecutors.onPresto;
+import static io.prestodb.tempto.assertions.QueryAssert.Row.row;
 import static io.prestodb.tempto.assertions.QueryAssert.assertThat;
 import static io.prestodb.tempto.query.QueryExecutor.defaultQueryExecutor;
 import static io.prestodb.tempto.query.QueryExecutor.query;
@@ -107,4 +113,41 @@ public void selectMetadataCatalogs()
                 .hasColumns(VARCHAR, VARCHAR)
                 .hasAnyRows();
     }
+
+    @Test(groups = SYSTEM_CONNECTOR)
+    public void selectJdbcColumns()
+    {
+        try {
+            String hiveSQL = "select table_name, column_name from system.jdbc.columns where table_cat = 'hive' AND table_schem = 'default'";
+            String icebergSQL = "select table_name, column_name from system.jdbc.columns where table_cat = 'iceberg' AND table_schem = 'default'";
+
+            List<QueryAssert.Row> preexistingHiveColumns = onPresto().executeQuery(hiveSQL).rows().stream()
+                    .map(list -> row(list.toArray()))
+                    .collect(Collectors.toList());
+
+            List<QueryAssert.Row> preexistingIcebergColumns = onPresto().executeQuery(icebergSQL).rows().stream()
+                    .map(list -> row(list.toArray()))
+                    .collect(Collectors.toList());
+
+            onPresto().executeQuery("CREATE TABLE hive.default.test_hive_system_jdbc_columns (_double DOUBLE)");
+            onPresto().executeQuery("CREATE TABLE iceberg.default.test_iceberg_system_jdbc_columns (_string VARCHAR, _integer INTEGER)");
+
+            assertThat(onPresto().executeQuery(hiveSQL))
+                    .containsOnly(ImmutableList.<QueryAssert.Row>builder()
+                            .addAll(preexistingHiveColumns)
+                            .add(row("test_hive_system_jdbc_columns", "_double"))
+                            .build());
+
+            assertThat(onPresto().executeQuery(icebergSQL))
+                    .containsOnly(ImmutableList.<QueryAssert.Row>builder()
+                            .addAll(preexistingIcebergColumns)
+                            .add(row("test_iceberg_system_jdbc_columns", "_string"))
+                            .add(row("test_iceberg_system_jdbc_columns", "_integer"))
+                            .build());
+        }
+        finally {
+            onPresto().executeQuery("DROP TABLE IF EXISTS hive.default.test_hive_system_jdbc_columns");
+            onPresto().executeQuery("DROP TABLE IF EXISTS iceberg.default.test_iceberg_system_jdbc_columns");
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -11,18 +11,17 @@`
`11`	`11`	`* See the License for the specific language governing permissions and`
`12`	`12`	`* limitations under the License.`
`13`	`13`	`*/`
`14`		`-package com.facebook.presto.iceberg;`
	`14`	`+package com.facebook.presto.hive;`
`15`	`15`
`16`	`16`	`import com.facebook.presto.spi.PrestoException;`
`17`		`-import com.facebook.presto.spi.SchemaTableName;`
`18`	`17`
`19`		`-import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_UNKNOWN_TABLE_TYPE;`
	`18`	`+import static com.facebook.presto.hive.HiveErrorCode.UNKNOWN_TABLE_TYPE;`
`20`	`19`
`21`	`20`	`public class UnknownTableTypeException`
`22`	`21`	`extends PrestoException`
`23`	`22`	`{`
`24`		`- public UnknownTableTypeException(SchemaTableName tableName)`
	`23`	`+ public UnknownTableTypeException(String message)`
`25`	`24`	`{`
`26`		`- super(ICEBERG_UNKNOWN_TABLE_TYPE, "Not an Iceberg table: " + tableName);`
	`25`	`+ super(UNKNOWN_TABLE_TYPE, message);`
`27`	`26`	`}`
`28`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@`
`15`	`15`
`16`	`16`	`import com.facebook.airlift.json.JsonCodec;`
`17`	`17`	`import com.facebook.airlift.json.smile.SmileCodec;`
	`18`	`+import com.facebook.airlift.log.Logger;`
`18`	`19`	`import com.facebook.presto.common.CatalogSchemaName;`
`19`	`20`	`import com.facebook.presto.common.Page;`
`20`	`21`	`import com.facebook.presto.common.Subfield;`
`@@ -389,6 +390,7 @@`
`389`	`390`	`public class HiveMetadata`
`390`	`391`	`implements TransactionalMetadata`
`391`	`392`	`{`
	`393`	`+ private static final Logger log = Logger.get(HiveMetadata.class);`
`392`	`394`	`public static final Set<String> RESERVED_ROLES = ImmutableSet.of("all", "default", "none");`
`393`	`395`	`public static final String REFERENCED_MATERIALIZED_VIEWS = "referenced_materialized_views";`
`394`	`396`
`@@ -674,7 +676,7 @@ private ConnectorTableMetadata getTableMetadata(Optional<Table> table, SchemaTab`
`674`	`676`	`}`
`675`	`677`
`676`	`678`	`if (isIcebergTable(table.get()) \|\| isDeltaLakeTable(table.get())) {`
`677`		`- throw new PrestoException(HIVE_UNSUPPORTED_FORMAT, format("Not a Hive table '%s'", tableName));`
	`679`	`+ throw new UnknownTableTypeException("Not a Hive table: " + tableName);`
`678`	`680`	`}`
`679`	`681`
`680`	`682`	`List<TableConstraint<String>> tableConstraints = metastore.getTableConstraints(metastoreContext, tableName.getSchemaName(), tableName.getTableName());`
`@@ -862,6 +864,9 @@ public Map<SchemaTableName, List<ColumnMetadata>> listTableColumns(ConnectorSess`
`862`	`864`	`catch (TableNotFoundException e) {`
`863`	`865`	`// table disappeared during listing operation`
`864`	`866`	`}`
	`867`	`+ catch (UnknownTableTypeException e) {`
	`868`	`+ log.warn(String.format("%s: Unknown table type of table %s", e.getMessage(), tableName));`
	`869`	`+ }`
`865`	`870`	`}`
`866`	`871`	`return columns.build();`
`867`	`872`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,6 @@`
`24`	`24`	`public enum IcebergErrorCode`
`25`	`25`	`implements ErrorCodeSupplier`
`26`	`26`	`{`
`27`		`- ICEBERG_UNKNOWN_TABLE_TYPE(0, EXTERNAL),`
`28`	`27`	`ICEBERG_INVALID_METADATA(1, EXTERNAL),`
`29`	`28`	`ICEBERG_TOO_MANY_OPEN_PARTITIONS(2, USER_ERROR),`
`30`	`29`	`ICEBERG_INVALID_PARTITION_VALUE(3, EXTERNAL),`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@`
`24`	`24`	`import com.facebook.presto.hive.HiveTypeTranslator;`
`25`	`25`	`import com.facebook.presto.hive.NodeVersion;`
`26`	`26`	`import com.facebook.presto.hive.TableAlreadyExistsException;`
	`27`	`+import com.facebook.presto.hive.UnknownTableTypeException;`
`27`	`28`	`import com.facebook.presto.hive.ViewAlreadyExistsException;`
`28`	`29`	`import com.facebook.presto.hive.metastore.Column;`
`29`	`30`	`import com.facebook.presto.hive.metastore.Database;`
`@@ -230,7 +231,7 @@ protected boolean tableExists(ConnectorSession session, SchemaTableName schemaTa`
`230`	`231`	`return false;`
`231`	`232`	`}`
`232`	`233`	`if (!isIcebergTable(hiveTable.get())) {`
`233`		`- throw new UnknownTableTypeException(schemaTableName);`
	`234`	`+ throw new UnknownTableTypeException("Not an Iceberg table: " + schemaTableName);`
`234`	`235`	`}`
`235`	`236`	`return true;`
`236`	`237`	`}`