>From Hussain Towaileb <[email protected]>: Attention is currently required from: Wail Alkowaileet. Hussain Towaileb has posted comments on this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751 )
Change subject: [ASTERIXDB-3255][EXT] Part1: Embed external filter values ...................................................................... Patch Set 4: Code-Review+2 (5 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/pushdown/PushdownProcessorsExecutor.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751/comment/e182f379_769631ce PS1, Line 119: ExternalDataPrefix.containsComputedFields(configuration) && Boolean.parseBoolean( : configuration.getOrDefault(ExternalDataConstants.KEY_EMBED_FILTER_VALUES, ExternalDataConstants.TRUE)); Should we contain all of this inside ExternalDataPrefix? Something like ExternalDaataPrefix.shouldEmbed(configuration)? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/filter/ExternalFilterValueEvaluator.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751/comment/7f94b56e_33f208cd PS4, Line 61: ArrayBackedValueStorage value, : AStringSerializerDeserializer stringSerDer What's the difference between those 2 and the ones defined in the class? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/filter/embedder/ExternalFilterValueEmbedder.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751/comment/341fb89f_00dec120 PS4, Line 82: shouldEmbed Does this have implementation anywhere? File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/filter/embedder/IExternalFilterValueEmbedder.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751/comment/823eaa15_07ac48c5 PS4, Line 26: IExternalFilterValueEmbedder minor: let's have docs for the methods in a follow up patch. File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/projection/ExternalDatasetProjectionFiltrationInfo.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751/comment/547118b3_44019c97 PS4, Line 150: embed-filter-value minor: embed-filter-value or embed-filter-values? Also, can use the constant in ExternalDataConstants? -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/17751 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: Ia61a62ab1390d9d87105898cf8c17f77c136c434 Gerrit-Change-Number: 17751 Gerrit-PatchSet: 4 Gerrit-Owner: Wail Alkowaileet <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-Attention: Wail Alkowaileet <[email protected]> Gerrit-Comment-Date: Fri, 08 Sep 2023 00:11:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
