Simplify getRawSystemTable in Delta Lake

trinodb · Jul 11, 2023 · 4347376 · 4347376
1 parent 002d707
commit 4347376
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 33 deletions.
diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
@@ -3015,40 +3015,42 @@ public Optional<SystemTable> getSystemTable(ConnectorSession session, SchemaTabl
                 .map(systemTable -> new ClassLoaderSafeSystemTable(systemTable, getClass().getClassLoader()));
     }
 
-    private Optional<SystemTable> getRawSystemTable(ConnectorSession session, SchemaTableName tableName)
+    private Optional<SystemTable> getRawSystemTable(ConnectorSession session, SchemaTableName systemTableName)
     {
-        if (DeltaLakeTableName.isDataTable(tableName.getTableName())) {
+        Optional<DeltaLakeTableType> tableType = DeltaLakeTableName.tableTypeFrom(systemTableName.getTableName());
+        if (tableType.isEmpty() || tableType.get() == DeltaLakeTableType.DATA) {
             return Optional.empty();
         }
 
-        // Only when dealing with an actual system table proceed to retrieve the table handle
-        String name = DeltaLakeTableName.tableNameFrom(tableName.getTableName());
-        ConnectorTableHandle tableHandle;
+        String tableName = DeltaLakeTableName.tableNameFrom(systemTableName.getTableName());
+        Optional<DeltaMetastoreTable> table;
         try {
-            tableHandle = getTableHandle(session, new SchemaTableName(tableName.getSchemaName(), name));
+            table = metastore.getTable(systemTableName.getSchemaName(), tableName);
         }
         catch (NotADeltaLakeTableException e) {
-            // avoid dealing with non Delta Lake tables
             return Optional.empty();
         }
-        if (tableHandle == null) {
-            return Optional.empty();
-        }
-        if (tableHandle instanceof CorruptedDeltaLakeTableHandle) {
+        if (table.isEmpty()) {
             return Optional.empty();
         }
-        DeltaLakeTableHandle handle = (DeltaLakeTableHandle) tableHandle;
 
-        Optional<DeltaLakeTableType> tableType = DeltaLakeTableName.tableTypeFrom(tableName.getTableName());
-        if (tableType.isEmpty()) {
-            return Optional.empty();
+        String tableLocation = table.get().location();
+        TableSnapshot tableSnapshot = getSnapshot(new SchemaTableName(systemTableName.getSchemaName(), tableName), tableLocation, session);
+        try {
+            transactionLogAccess.getMetadataEntry(tableSnapshot, session);
         }
-        SchemaTableName systemTableName = new SchemaTableName(tableName.getSchemaName(), DeltaLakeTableName.tableNameWithType(name, tableType.get()));
+        catch (TrinoException e) {
+            if (e.getErrorCode().equals(DELTA_LAKE_INVALID_SCHEMA.toErrorCode())) {
+                return Optional.empty();
+            }
+            throw e;
+        }
+
         return switch (tableType.get()) {
             case DATA -> throw new VerifyException("Unexpected DATA table type"); // Handled above.
             case HISTORY -> Optional.of(new DeltaLakeHistoryTable(
                     systemTableName,
-                    getCommitInfoEntries(handle.getSchemaTableName(), handle.getLocation(), session),
+                    getCommitInfoEntries(tableLocation, session),
                     typeManager));
         };
     }
@@ -3129,7 +3131,7 @@ public DeltaLakeMetastore getMetastore()
         return metastore;
     }
 
-    private List<CommitInfoEntry> getCommitInfoEntries(SchemaTableName table, String tableLocation, ConnectorSession session)
+    private List<CommitInfoEntry> getCommitInfoEntries(String tableLocation, ConnectorSession session)
     {
         TrinoFileSystem fileSystem = fileSystemFactory.create(session);
         try {
@@ -3142,7 +3144,7 @@ private List<CommitInfoEntry> getCommitInfoEntries(SchemaTableName table, String
             throw e;
         }
         catch (IOException | RuntimeException e) {
-            throw new TrinoException(DELTA_LAKE_INVALID_SCHEMA, "Error getting commit info entries for " + table, e);
+            throw new TrinoException(DELTA_LAKE_INVALID_SCHEMA, "Error getting commit info entries from " + tableLocation, e);
         }
     }
 

diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableName.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableName.java
@@ -22,7 +22,6 @@
 
 import static io.trino.plugin.deltalake.DeltaLakeTableType.DATA;
 import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
-import static java.util.Objects.requireNonNull;
 
 public final class DeltaLakeTableName
 {
@@ -32,12 +31,6 @@ private DeltaLakeTableName() {}
             "(?<table>[^$@]+)" +
             "(?:\\$(?<type>[^@]+))?");
 
-    public static String tableNameWithType(String tableName, DeltaLakeTableType tableType)
-    {
-        requireNonNull(tableName, "tableName is null");
-        return tableName + "$" + tableType.name().toLowerCase(Locale.ENGLISH);
-    }
-
     public static String tableNameFrom(String name)
     {
         Matcher match = TABLE_PATTERN.matcher(name);

diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeBasic.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeBasic.java
@@ -377,6 +377,7 @@ private void testCorruptedTableLocation(String tableName, Path tableLocation, bo
 
         // Assert queries fail cleanly
         assertQueryFails("TABLE " + tableName, "Metadata not found in transaction log for tpch." + tableName);
+        assertQueryFails("SELECT * FROM \"" + tableName + "$history\"", ".* Table '.*\\$history' does not exist");
         assertQueryFails("SELECT * FROM " + tableName + " WHERE false", "Metadata not found in transaction log for tpch." + tableName);
         assertQueryFails("SELECT 1 FROM " + tableName + " WHERE false", "Metadata not found in transaction log for tpch." + tableName);
         assertQueryFails("SHOW CREATE TABLE " + tableName, "Metadata not found in transaction log for tpch." + tableName);

diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableName.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeTableName.java
@@ -71,13 +71,6 @@ public void testTableTypeFrom()
         assertEquals(DeltaLakeTableName.tableTypeFrom("abc$invalid"), Optional.empty());
     }
 
-    @Test
-    public void testTableNameWithType()
-    {
-        assertEquals(DeltaLakeTableName.tableNameWithType("abc", DATA), "abc$data");
-        assertEquals(DeltaLakeTableName.tableNameWithType("abc", HISTORY), "abc$history");
-    }
-
     private static void assertNoValidTableType(String inputName)
     {
         Assertions.assertThat(DeltaLakeTableName.tableTypeFrom(inputName))