>From Ali Alsuliman <[email protected]>: Attention is currently required from: Preetham Poluparthi.
Ali Alsuliman has posted comments on this change by Preetham Poluparthi. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281?usp=email ) Change subject: [ASTERIXDB-3632] Add Array Index recommendations to Index Advisor ...................................................................... Patch Set 16: (8 comments) File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/AdviseIndexRule.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/e8ce13be_3e688c64?usp=email : PS16, Line 149: indexAdvisor.addRecommendedAdviseString(new ValueAdvise( If ValueAdvise and ArrayAdvise are only to be used to generate the DDL, then there is not much value in creating classes with inheritance and creating objects just to get the string. You could simply just have static methods for each calling other common methods whenever needed. https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/015991a1_c9a46dc6?usp=email : PS16, Line 235: for (Index.ArrayIndexElement arrayIndexElement : arrayIndexDetails.getElementList()) { Try these examples: CREATE INDEX i1 ON ds1(UNNEST a : string) EXCLUDE UNKNOWN KEY; CREATE INDEX i2 ON ds1(UNNEST b SELECT x : int) EXCLUDE UNKNOWN KEY; CREATE INDEX i3 ON ds1(UNNEST b.c UNNEST d.e UNNEST t SELECT x.y : string, q.w: int, u: int) EXCLUDE UNKNOWN KEY; CREATE INDEX i4 ON ds1(p: int, (UNNEST b.c UNNEST d.e SELECT x.y : string, q.w: int), z.m: double) EXCLUDE UNKNOWN KEY; File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/FakeIndex.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/17315f44_5e33e6a3?usp=email : PS16, Line 47: List<List<String>> unnestList, List<List<String>> projectList, IAType itemType) { Is this really a single element while projectList is a list? File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/FakeIndexProvider.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/0fb1f009_bf92374c?usp=email : PS16, Line 85: continue; There is no point in doing "continue" since if datasetIndexes == null, then it's going to be null for all unnestFilterConditions. Shouldn't you continue the outer-loop instead of this for-loop? or actually move this check outside? https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/86a12963_ed7fb1e4?usp=email : PS16, Line 94: if (unnestToProjectListMap.containsKey(unnestList)) { I would do it like this: // look-up the map only once Index.ArrayIndexElement arrayIndexElement = unnestToProjectListMap.get(unnestList); if (arrayIndexElement != null) { // case where it's an existing unnest if (arrayIndexElement.getProjectList().put(projectList) == null) { ... } } else { // case where it's a new unnest List<List<String>> projectListWrapper = new ArrayList<>(); ... } https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/23d2e4ad_3bbe19cf?usp=email : PS16, Line 96: if (!arrayIndexElement.getProjectList().contains(projectList)) { Do this if it's equivalent to what you want to do: if (arrayIndexElement.getProjectList().put(projectList) == null) { arrayIndexElement.getTypeList().add(iaType); } File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/cbo/indexadvisor/UnnestFilterCondition.java: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/3a53f6ae_0cd59694?usp=email : PS16, Line 28: // private final Index.ArrayIndexElement arrayIndexElement; Remove if not needed File asterixdb/asterix-app/src/test/resources/runtimets/results/cbo-join/index-advisor/array-simple-advise/array-simple-advise.6.adm: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281/comment/9aa0c59d_a3b37e30?usp=email : PS16, Line 1: [{"#operator":"Advise","advice":{"#operator":"IndexAdvice","adviseinfo":{"current_indexes":[],"recommended_indexes":{"indexes":[{"index_statement":"CREATE INDEX array_idx_items ON `Default`.`test`.`A`(UNNEST `items` SELECT `qty`: bigint, SELECT `price`: double);"}]}}}}] Isn't this supposed to be: SELECT `qty`: bigint, `price`: double -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20281?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I9f7b70321a7ec831b4880437871af9749d93d0c4 Gerrit-Change-Number: 20281 Gerrit-PatchSet: 16 Gerrit-Owner: Preetham Poluparthi <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Attention: Preetham Poluparthi <[email protected]> Gerrit-Comment-Date: Mon, 22 Sep 2025 22:44:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
