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 index 9abdff60d62b..3ad5e60acfb6 100644 --- 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 @@ -71,12 +71,21 @@ public static DeltaLakeTableName from(String name) String table = match.group("table"); String typeString = match.group("type"); - DeltaLakeTableType type = DeltaLakeTableType.DATA; - if (typeString != null) { + DeltaLakeTableType type; + if (typeString == null) { + type = DATA; + } + else { + type = null; try { - type = DeltaLakeTableType.valueOf(typeString.toUpperCase(Locale.ENGLISH)); + DeltaLakeTableType parsedType = DeltaLakeTableType.valueOf(typeString.toUpperCase(Locale.ENGLISH)); + if (parsedType != DATA) { + type = parsedType; + } + } + catch (IllegalArgumentException ignored) { } - catch (IllegalArgumentException e) { + if (type == null) { throw new TrinoException(NOT_SUPPORTED, format("Invalid Delta Lake table name (unknown type '%s'): %s", typeString, name)); } } @@ -105,7 +114,12 @@ public static Optional tableTypeFrom(String name) return Optional.of(DATA); } try { - return Optional.of(DeltaLakeTableType.valueOf(typeString.toUpperCase(Locale.ENGLISH))); + DeltaLakeTableType parsedType = DeltaLakeTableType.valueOf(typeString.toUpperCase(Locale.ENGLISH)); + if (parsedType == DATA) { + // $data cannot be encoded in table name + return Optional.empty(); + } + return Optional.of(parsedType); } catch (IllegalArgumentException e) { return Optional.empty(); @@ -119,15 +133,6 @@ public static boolean isDataTable(String name) throw new TrinoException(NOT_SUPPORTED, "Invalid Delta Lake table name: " + name); } String typeString = match.group("type"); - if (typeString == null) { - return true; - } - try { - DeltaLakeTableType type = DeltaLakeTableType.valueOf(typeString.toUpperCase(Locale.ENGLISH)); - return type == DATA; - } - catch (IllegalArgumentException e) { - return false; - } + return typeString == null; } } 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 index 86a9cbee93af..c592b0be6430 100644 --- 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 @@ -31,9 +31,9 @@ public class TestDeltaLakeTableName public void testFrom() { assertFrom("abc", "abc", DATA); - assertFrom("abc$data", "abc", DATA); assertFrom("abc$history", "abc", DeltaLakeTableType.HISTORY); + assertInvalid("abc$data", "Invalid Delta Lake table name (unknown type 'data'): abc$data"); assertInvalid("abc@123", "Invalid Delta Lake table name: abc@123"); assertInvalid("abc@xyz", "Invalid Delta Lake table name: abc@xyz"); assertInvalid("abc$what", "Invalid Delta Lake table name (unknown type 'what'): abc$what"); @@ -45,8 +45,8 @@ public void testFrom() public void testIsDataTable() { assertTrue(DeltaLakeTableName.isDataTable("abc")); - assertTrue(DeltaLakeTableName.isDataTable("abc$data")); + assertFalse(DeltaLakeTableName.isDataTable("abc$data")); // it's invalid assertFalse(DeltaLakeTableName.isDataTable("abc$history")); assertFalse(DeltaLakeTableName.isDataTable("abc$invalid")); } @@ -64,7 +64,7 @@ public void testTableNameFrom() public void testTableTypeFrom() { assertEquals(DeltaLakeTableName.tableTypeFrom("abc"), Optional.of(DATA)); - assertEquals(DeltaLakeTableName.tableTypeFrom("abc$data"), Optional.of(DATA)); + assertEquals(DeltaLakeTableName.tableTypeFrom("abc$data"), Optional.empty()); // it's invalid assertEquals(DeltaLakeTableName.tableTypeFrom("abc$history"), Optional.of(HISTORY)); assertEquals(DeltaLakeTableName.tableTypeFrom("abc$invalid"), Optional.empty()); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java index 790088da7753..697d51344a7c 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergTableName.java @@ -71,12 +71,21 @@ public static IcebergTableName from(String name) String table = match.group("table"); String typeString = match.group("type"); - TableType type = DATA; - if (typeString != null) { + TableType type; + if (typeString == null) { + type = DATA; + } + else { + type = null; try { - type = TableType.valueOf(typeString.toUpperCase(ENGLISH)); + TableType parsedType = TableType.valueOf(typeString.toUpperCase(ENGLISH)); + if (parsedType != DATA) { + type = parsedType; + } } - catch (IllegalArgumentException e) { + catch (IllegalArgumentException ignored) { + } + if (type == null) { throw new TrinoException(NOT_SUPPORTED, format("Invalid Iceberg table name (unknown type '%s'): %s", typeString, name)); } } @@ -105,7 +114,12 @@ public static Optional tableTypeFrom(String name) return Optional.of(DATA); } try { - return Optional.of(TableType.valueOf(typeString.toUpperCase(ENGLISH))); + TableType parsedType = TableType.valueOf(typeString.toUpperCase(ENGLISH)); + if (parsedType == DATA) { + // $data cannot be encoded in table name + return Optional.empty(); + } + return Optional.of(parsedType); } catch (IllegalArgumentException e) { return Optional.empty(); @@ -119,17 +133,6 @@ public static boolean isDataTable(String name) throw new TrinoException(NOT_SUPPORTED, "Invalid Iceberg table name: " + name); } String typeString = match.group("type"); - if (typeString == null) { - return true; - } - else { - try { - TableType type = TableType.valueOf(typeString.toUpperCase(ENGLISH)); - return type == DATA; - } - catch (IllegalArgumentException e) { - return false; - } - } + return typeString == null; } } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableName.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableName.java index fab3c9c18b26..0218986b31dd 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableName.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergTableName.java @@ -29,10 +29,10 @@ public class TestIcebergTableName public void testFrom() { assertFrom("abc", "abc", TableType.DATA); - assertFrom("abc$data", "abc", TableType.DATA); assertFrom("abc$history", "abc", TableType.HISTORY); assertFrom("abc$snapshots", "abc", TableType.SNAPSHOTS); + assertInvalid("abc$data", "Invalid Iceberg table name (unknown type 'data'): abc$data"); assertInvalid("abc@123", "Invalid Iceberg table name: abc@123"); assertInvalid("abc@xyz", "Invalid Iceberg table name: abc@xyz"); assertInvalid("abc$what", "Invalid Iceberg table name (unknown type 'what'): abc$what"); @@ -48,8 +48,8 @@ public void testFrom() public void testIsDataTable() { assertTrue(IcebergTableName.isDataTable("abc")); - assertTrue(IcebergTableName.isDataTable("abc$data")); + assertFalse(IcebergTableName.isDataTable("abc$data")); // it's invalid assertFalse(IcebergTableName.isDataTable("abc$history")); assertFalse(IcebergTableName.isDataTable("abc$invalid")); } @@ -67,7 +67,7 @@ public void testTableNameFrom() public void testTableTypeFrom() { assertEquals(IcebergTableName.tableTypeFrom("abc"), Optional.of(TableType.DATA)); - assertEquals(IcebergTableName.tableTypeFrom("abc$data"), Optional.of(TableType.DATA)); + assertEquals(IcebergTableName.tableTypeFrom("abc$data"), Optional.empty()); // it's invalid assertEquals(IcebergTableName.tableTypeFrom("abc$history"), Optional.of(TableType.HISTORY)); assertEquals(IcebergTableName.tableTypeFrom("abc$invalid"), Optional.empty()); diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java index 204e699f1e41..0291e6247e07 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java @@ -4675,6 +4675,17 @@ public void testWrittenStats() } } + /** + * Some connectors support system table denoted with $-suffix. Ensure no connector exposes table_name$data + * directly to users, as it would mean the same thing as table_name itself. + */ + @Test + public void testNoDataSystemTable() + { + assertQuerySucceeds("TABLE nation"); + assertQueryFails("TABLE \"nation$data\"", "line 1:1: Table '\\w+.\\w+.nation\\$data' does not exist"); + } + @Test(dataProvider = "testColumnNameDataProvider") public void testColumnName(String columnName) {