gaborkaszab commented on code in PR #17134: URL: https://github.com/apache/iceberg/pull/17134#discussion_r3577697278
########## core/src/main/java/org/apache/iceberg/MetadataTablesTable.java: ########## @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg; + +import java.util.Arrays; +import org.apache.iceberg.types.Types; + +/** A {@link Table} implementation that exposes a table's metadata tables as rows. */ +public class MetadataTablesTable extends BaseMetadataTable { + MetadataTablesTable(Table table) { + this(table, table.name() + "." + MetadataTableType.METADATA_TABLES.tableName()); Review Comment: This is some middle ground between the intent of this PR and the intent of the follow-up refactor PR we discussed earlier. For this PR I'd follow the existing pattern and use string literal here for the metadata table name. With this, we can remove the relevant code from the enum too. If you decide to implement the follow-up refactor, then you will do these changes anyway. Alternatively, the refactor PR could be covered first and then after a rebase we can use that functionality here. ########## core/src/main/java/org/apache/iceberg/MetadataTablesTable.java: ########## @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg; + +import java.util.Arrays; +import org.apache.iceberg.types.Types; + +/** A {@link Table} implementation that exposes a table's metadata tables as rows. */ +public class MetadataTablesTable extends BaseMetadataTable { + MetadataTablesTable(Table table) { + this(table, table.name() + "." + MetadataTableType.METADATA_TABLES.tableName()); + } + + MetadataTablesTable(Table table, String name) { + super(table, name); + } + + private static final Schema METADATA_TABLES_SCHEMA = + new Schema(Types.NestedField.required(1, "metadata_table_name", Types.StringType.get())); + + @Override + public TableScan newScan() { + return new MetadataTablesScan(table()); + } + + @Override + public Schema schema() { + return METADATA_TABLES_SCHEMA; + } + + @Override + MetadataTableType metadataTableType() { + return MetadataTableType.METADATA_TABLES; + } + + private DataTask task(BaseTableScan scan) { + String location = table().operations().current().metadataFileLocation(); + return StaticDataTask.of( + table().io().newInputFile(location != null ? location : scan.table().location()), + schema(), + scan.schema(), + Arrays.asList(MetadataTableType.values()), + metadataType -> StaticDataTask.Row.of(metadataType.tableName())); Review Comment: Here, I'd simply get the enum values and convert them to the desired output format here. ########## core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java: ########## @@ -1833,6 +1833,26 @@ public void testDeleteFilesTableScanOnBranch() throws IOException { .isEqualTo(2); } + @TestTemplate + public void testMetadataTablesTableScan() throws IOException { + table.newFastAppend().appendFile(FILE_A).commit(); + + Table metadataTablesTable = new MetadataTablesTable(table); + List<String> actualTables = Lists.newArrayList(); + try (CloseableIterable<FileScanTask> tasks = metadataTablesTable.newScan().planFiles()) { + assertThat(tasks).hasSize(1); Review Comment: I don't think this assert is needed. In fact, for tests I think this part could be simplified a lot. Like: ``` metadataTablesTable.newScan().planFiles().asDataTask.rows() ``` ########## core/src/main/java/org/apache/iceberg/MetadataTableType.java: ########## @@ -45,4 +55,14 @@ public static MetadataTableType from(String name) { return null; } } + + public String tableName() { + return this.tableName; + } + + public static List<String> tableNames() { Review Comment: This is only used in a test to construct the expected values. I wouldn't introduce such a function here. ########## docs/docs/spark-queries.md: ########## @@ -340,6 +340,32 @@ To inspect a table's history, snapshots, and other metadata, Iceberg supports me Metadata tables are identified by adding the metadata table name after the original table name. For example, history for `db.table` is read using `db.table.history`. +### Metadata Tables +To list the metadata tables supported by a table: + +```sql +SELECT * FROM prod.db.table.metadata_tables; +``` + +| metadata_table_name | +|----------------------| +| entries | Review Comment: This is in the order of how they are defined in the enum. Shouldn't this be in alphabetical order when showing for the users? I know we can add an ORDER BY to the query, but seems low effort to do it under the hood ########## core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java: ########## @@ -1833,6 +1833,26 @@ public void testDeleteFilesTableScanOnBranch() throws IOException { .isEqualTo(2); } + @TestTemplate + public void testMetadataTablesTableScan() throws IOException { + table.newFastAppend().appendFile(FILE_A).commit(); Review Comment: Is it necessary to add a snapshot to the table? Should work for empty tables too. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
