From ccb5a8516aadcc33b480af3778ddcca7f2661a37 Mon Sep 17 00:00:00 2001 From: Andrey Shitov Date: Thu, 30 May 2024 13:32:45 +0300 Subject: [PATCH] IMPALA-13035 --- .../org/apache/impala/analysis/Analyzer.java | 9 ++++-- .../java/org/apache/impala/analysis/Path.java | 2 +- .../catalog/iceberg/IcebergMetadataTable.java | 32 ++++++++++++++++--- .../impala/service/DescribeResultFactory.java | 6 ++-- .../org/apache/impala/service/Frontend.java | 4 +-- .../QueryTest/iceberg-metadata-tables.test | 15 +++++++-- 6 files changed, 52 insertions(+), 16 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java index b825f88d8d..4212cce892 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java +++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java @@ -3619,7 +3619,7 @@ public void addAccessEvent(TAccessEvent event) { */ public FeTable getTable(TableName tblName, boolean mustExist) throws AnalysisException, TableLoadingException { - if (IcebergMetadataTable.isIcebergMetadataTable(tblName.toPath())) { + if (IcebergMetadataTable.isIcebergMetadataTable(tblName.toPath(), this)) { return getMetadataVirtualTable(tblName.toPath()); } FeTable table = globalState_.stmtTableCache.tables.get(tblName); @@ -3671,7 +3671,8 @@ public void addVirtualTable(VirtualTable virtTable) { */ public FeTable getMetadataVirtualTable(List tblRefPath) throws AnalysisException { - Preconditions.checkArgument(IcebergMetadataTable.isIcebergMetadataTable(tblRefPath)); + Preconditions.checkArgument( + IcebergMetadataTable.canBeIcebergMetadataTable(tblRefPath)); try { TableName catalogTableName = new TableName(tblRefPath.get(0), tblRefPath.get(1)); @@ -3683,8 +3684,10 @@ public FeTable getMetadataVirtualTable(List tblRefPath) if (catalogTable instanceof IcebergMetadataTable || catalogTable == null) { return catalogTable; } + Preconditions.checkState(catalogTable instanceof FeIcebergTable); + FeIcebergTable catalogIceTable = (FeIcebergTable) catalogTable; IcebergMetadataTable virtualTable = - new IcebergMetadataTable(catalogTable, tblRefPath.get(2)); + new IcebergMetadataTable(catalogIceTable, tblRefPath.get(2)); getStmtTableCache().tables.put(catalogTableName, catalogTable); getStmtTableCache().tables.put(virtualTableName, virtualTable); return virtualTable; diff --git a/fe/src/main/java/org/apache/impala/analysis/Path.java b/fe/src/main/java/org/apache/impala/analysis/Path.java index 5083527c6d..a78f6b1d44 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Path.java +++ b/fe/src/main/java/org/apache/impala/analysis/Path.java @@ -350,7 +350,7 @@ public static List getCandidateTables(List path, String sessi String dbName = (tblNameIdx == 0) ? sessionDb : path.get(0); String tblName = path.get(tblNameIdx); String vTblName = null; - if (IcebergMetadataTable.isIcebergMetadataTable(path)) { + if (IcebergMetadataTable.canBeIcebergMetadataTable(path)) { vTblName = path.get(2); } result.add(new TableName(dbName, tblName, vTblName)); diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java index 3d417eef94..ce3f333f34 100644 --- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java +++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java @@ -25,6 +25,7 @@ import org.apache.iceberg.MetadataTableUtils; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; +import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.TableName; import org.apache.impala.catalog.CatalogObject.ThriftObjectType; import org.apache.impala.catalog.Column; @@ -59,11 +60,10 @@ public class IcebergMetadataTable extends VirtualTable { // Name of the metadata table. private String metadataTableName_; - public IcebergMetadataTable(FeTable baseTable, String metadataTableTypeStr) + public IcebergMetadataTable(FeIcebergTable baseTable, String metadataTableTypeStr) throws ImpalaRuntimeException { super(null, baseTable.getDb(), baseTable.getName(), baseTable.getOwnerUser()); - Preconditions.checkArgument(baseTable instanceof FeIcebergTable); - baseTable_ = (FeIcebergTable) baseTable; + baseTable_ = baseTable; metadataTableName_ = metadataTableTypeStr.toUpperCase(); MetadataTableType type = MetadataTableType.from(metadataTableTypeStr.toUpperCase()); Preconditions.checkNotNull(type); @@ -136,7 +136,31 @@ private List getTColumnDescriptors() { /** * Returns true if the table ref is referring to a valid metadata table. */ - public static boolean isIcebergMetadataTable(List tblRefPath) { + public static boolean isIcebergMetadataTable(List tblRefPath, + Analyzer analyzer) { + if (!canBeIcebergMetadataTable(tblRefPath)) return false; + + TableName virtualTableName = new TableName(tblRefPath.get(0), + tblRefPath.get(1), tblRefPath.get(2)); + // The catalog table (the base of the virtual table) has been loaded and cached + // under the name of the virtual table. + FeTable catalogTable = analyzer.getStmtTableCache().tables.get(virtualTableName); + // If the metadata table has already been analyzed in the query, the table cache will + // return the virtual table, not the base table. + return catalogTable instanceof FeIcebergTable || + catalogTable instanceof IcebergMetadataTable; + } + + /** + * Returns true if the path could refer to an Iceberg metadata table in a syntactically + * correct way (also checking that the name of the metadata table is valid). Does not + * check whether the base table is an Iceberg table, so the path is not guaranteed to + * actually refer to a valid Iceberg metadata table. + * + * This function can be called before analysis is done, when isIcebergMetadataTable() + * cannot be called. + */ + public static boolean canBeIcebergMetadataTable(List tblRefPath) { if (tblRefPath == null) return false; if (tblRefPath.size() < 3) return false; String vTableName = tblRefPath.get(2).toUpperCase(); diff --git a/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java b/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java index 6ce58e5c11..8880da3d48 100644 --- a/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java +++ b/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java @@ -362,9 +362,9 @@ public static TDescribeResult buildIcebergDescribeMinimalResult(List col * it is simpler to re-create this object than to extract those from a new * org.apache.iceberg.Table object or to send it over. */ - public static TDescribeResult buildIcebergMetadataDescribeMinimalResult(FeTable table, - String vTableName) throws ImpalaRuntimeException { + public static TDescribeResult buildIcebergMetadataDescribeMinimalResult( + FeIcebergTable table, String vTableName) throws ImpalaRuntimeException { IcebergMetadataTable metadataTable = new IcebergMetadataTable(table, vTableName); return buildIcebergDescribeMinimalResult(metadataTable.getColumns()); } -} \ No newline at end of file +} diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java index 4da39d05a7..f3102390f0 100644 --- a/fe/src/main/java/org/apache/impala/service/Frontend.java +++ b/fe/src/main/java/org/apache/impala/service/Frontend.java @@ -1797,8 +1797,8 @@ private TDescribeResult doDescribeTable(TTableName tableName, return DescribeResultFactory.buildIcebergDescribeMinimalResult(filteredColumns); } else { Preconditions.checkArgument(vTableName != null); - return DescribeResultFactory.buildIcebergMetadataDescribeMinimalResult(table, - vTableName); + return DescribeResultFactory.buildIcebergMetadataDescribeMinimalResult( + (FeIcebergTable) table, vTableName); } } else { return DescribeResultFactory.buildDescribeMinimalResult( diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test index 50c3fe2eae..cfd2782289 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test +++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test @@ -173,7 +173,7 @@ select * from $DATABASE.empty_ice_tbl.entries; INT,BIGINT,BIGINT,BIGINT,STRING,STRING #### -# Test 2 : Test select list +# Test select list #### ==== ---- QUERY @@ -401,7 +401,7 @@ AnalysisException: FOR SYSTEM_VERSION AS OF clause is only supported for Iceberg ==== #### -# Test 9 : Use-cases +# Use-cases #### ==== ---- QUERY @@ -430,11 +430,20 @@ row_regex:[1-9]\d*|0,'$NAMENODE/test-warehouse/iceberg_test/hadoop_catalog/ice/i ---- TYPES INT,STRING,BIGINT +#### +# Test querying a metadata table of a non-Iceberg table. +#### +==== +---- QUERY +select * from functional_parquet.alltypes.`files`; +---- CATCH +AnalysisException: Could not resolve table reference: 'functional_parquet.alltypes.files' +==== + #### # Invalid operations # In most cases the parser catches the table reference. #### -==== ---- QUERY describe formatted functional_parquet.iceberg_query_metadata.snapshots; ---- CATCH