>From Wael Alkowaileet <[email protected]>: Wael Alkowaileet has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984 )
Change subject: [ASTERIXDB-2753][EXT] Support reading Parquet from S3 ...................................................................... Patch Set 12: Code-Review+1 (7 comments) The licenses issue has been addressed https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/10//COMMIT_MSG Commit Message: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/10//COMMIT_MSG@15 PS10, Line 15: Parqeut > Parquet Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/java/org/apache/asterix/common/BinaryFileConverterUtil.java File asterixdb/asterix-app/src/test/java/org/apache/asterix/common/BinaryFileConverterUtil.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/java/org/apache/asterix/common/BinaryFileConverterUtil.java@36 PS7, Line 36: BinaryFileConverterUtil > Are these utils only used in tests? If so, can this be moved in a test > package? Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.11.ddl.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.11.ddl.sqlpp: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.11.ddl.sqlpp@34 PS7, Line 34: CREATE EXTERNAL DATASET ParquetDataset(ParquetType) USING %adapter% > Can you move this CREATE EXTERNAL DATASET into expression-pushdown.01.ddl. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.12.update.sqlpp File asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.12.update.sqlpp: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/external-dataset/common/parquet/expression-pushdown/expression-pushdown.12.update.sqlpp@19 PS7, Line 19: /* > This file does nothing Let's remove it. Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/results/external-dataset/common/parquet/expression-pushdown/expression-pushdown.1.json File asterixdb/asterix-app/src/test/resources/runtimets/results/external-dataset/common/parquet/expression-pushdown/expression-pushdown.1.json: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-app/src/test/resources/runtimets/results/external-dataset/common/parquet/expression-pushdown/expression-pushdown.1.json@1 PS7, Line 1: { "id": 8, "age": 10 } > 1. […] Done https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataConstants.java@320 PS7, Line 320: fs.s3a.access.ke > For consistency sake, can we call this "fs.s3a.access.key. […] Correct. It is Hadoop-AWS specific property name. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984/7/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/ExternalDataUtils.java@969 PS7, Line 969: ExternalDataUtils.AwsS3. > Is this needed? The listS3Objects is in the same class. Done. Good catch! Fixed bunch of those. -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/8984 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: I7a8f1a9dc31d8b4af508e521d010e2ed10feb7dd Gerrit-Change-Number: 8984 Gerrit-PatchSet: 12 Gerrit-Owner: Wael Alkowaileet <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Wael Alkowaileet <[email protected]> Gerrit-CC: Till Westmann <[email protected]> Gerrit-Comment-Date: Tue, 29 Jun 2021 00:24:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Hussain Towaileb <[email protected]> Comment-In-Reply-To: Till Westmann <[email protected]> Comment-In-Reply-To: Dmitry Lychagin <[email protected]> Gerrit-MessageType: comment
