>From Murtadha Hubail <[email protected]>: Attention is currently required from: Peeyush Gupta, Ali Alsuliman, Michael Blow, Hussain Towaileb. Murtadha Hubail has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272 )
Change subject: [ASTERIXDB-3634][EXT]: Add support to Iceberg ...................................................................... Patch Set 3: (8 comments) File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/3b6fb192_32e0d56e PS3, Line 240: for (Map.Entry<String, IAdmNode> me : withObjectNode.getFields()) { feels like this logic can be extracted to a util outside of this class https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/04897827_1715dd93 PS3, Line 261: fieldName + "#" + i++ do we actually store them this way in our metadata catalog? File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CatalogConfig.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/4245f68e_b1d17f74 PS3, Line 35: IcebergCatalogSource You can potentially use Optional<IcebergCatalogSource> here File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/statement/catalog/ICatalogDetailsDecl.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/2f84ef73_0699b00c PS3, Line 21: ICatalogDetailsDecl I know you are just following the existing pattern, but in this case, we might not need this empty interface for catalog or you should add some base methods here. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/api/IMetadataManager.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/ee498358_a383048c PS3, Line 928: catalog make the parameter name consist between get and drop (i.e., catalog or catalogName). In the next interface, they are catalogName File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/CatalogEntity.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/b32713d9_9c746229 PS3, Line 103: FIELD_NAME_CATALOG_NAME we should seriously start considering adding a uuid for our entities to allow renaming entities, but you can consider that for another patch. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataRecordTypes.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/da5c28f8_53b8defa PS3, Line 316: FIELD_NAME_LAST_REFRESH_TIME, : FIELD_NAME_TRANSACTION_STATE }, Are you planning on adding a catalog refresh concept? if not, we you should remove these two fields. File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/IcebergCatalog.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272/comment/f6cd10e6_54f20178 PS3, Line 26: 2 why not 1? -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20272 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: If825dfc3241f72fcece03874662fc2dc8f553920 Gerrit-Change-Number: 20272 Gerrit-PatchSet: 3 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-CC: Michael Blow <[email protected]> Gerrit-Attention: Peeyush Gupta <[email protected]> Gerrit-Attention: Ali Alsuliman <[email protected]> Gerrit-Attention: Michael Blow <[email protected]> Gerrit-Attention: Hussain Towaileb <[email protected]> Gerrit-Comment-Date: Thu, 04 Sep 2025 14:21:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
