Yingyi Bu has posted comments on this change. Change subject: Fix Indexing on Open fields and Meta fields ......................................................................
Patch Set 3: (19 comments) Here are some high level comments: 1. It looks you don't change cast-record etc. Is the patch able to deal with open index on a field where some rows have a MISSING/NULL value on the field. For example change-feed-with-meta-pk-in-meta-open-index-in-value.1.ddl.aql What if some records do not have the id field? 2. Can you add optimizer tests to pair with each runtime test to verify that index access path is used? 3. Does index join work? Can you add runtime/optimizer test cases to verify that. 4. Can you add a test case to create an index on a composite of meta part fields and regular fields? 5. Some SonarQube comments still make sense. 6. It looks your Code style format is different the default one? Or my local one is different... E.g., QueryTranslator. Detailed comments are inlined. https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/IntroduceSecondaryIndexInsertDeleteRule.java: Line 778: new MutableObject<ILogicalExpression>(new VariableReferenceExpression(secondaryKeyVar))); rename isNullFuncExpr -> isUnknownFuncExpr https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: Line 480: .setPrevAdditionalNonFilteringVars(Collections.singletonList(context.newVar())); wrap that with a new ArrayList<>(....) since single list doesn't allow you add items. Line 482: Collections.singletonList(targetDatasource.getMetaItemType())); wrap that with a new ArrayList<>(....) since single list doesn't allow you add items. https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/file/SecondaryIndexOperationsHelper.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/file/SecondaryIndexOperationsHelper.java: Line 467: IScalarEvaluatorFactory isNullEvalFactory = rename: isNullEvalFactory -> isUnknownEvalFactory https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-open-index-in-meta/change-feed-with-meta-open-index-in-meta.1.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-open-index-in-meta/change-feed-with-meta-open-index-in-meta.1.ddl.aql: Line 23: * Date : 24th Feb 2016 change the date? Line 58: there is no data ingested for this case? It looks the feed is not connected. (There is an inconsistency with the description:-)) https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-open-index-in-meta/change-feed-with-meta-open-index-in-meta.2.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-open-index-in-meta/change-feed-with-meta-open-index-in-meta.2.ddl.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.1.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.1.ddl.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.3.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.3.ddl.aql: Line 22: * Date : 24th Feb 2016 change the date. https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.4.query.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.4.query.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.5.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-after-ingest/change-feed-with-meta-pk-in-meta-index-after-ingest.5.ddl.aql: Line 22: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.1.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.1.ddl.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.2.update.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.2.update.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.3.query.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.3.query.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.4.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-index-in-meta/change-feed-with-meta-pk-in-meta-index-in-meta.4.ddl.aql: Line 22: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.1.ddl.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.1.ddl.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.2.update.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.2.update.aql: Line 22: * Expected Res : Success change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.3.query.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/feeds/change-feed-with-meta-pk-in-meta-open-index-in-value/change-feed-with-meta-pk-in-meta-open-index-in-value.3.query.aql: Line 23: * Date : 24th Feb 2016 change the date https://asterix-gerrit.ics.uci.edu/#/c/930/3/asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml File asterixdb/asterix-app/src/test/resources/runtimets/testsuite.xml: Line 190: <test-case FilePath="external-library"> why move the test group "external-library" down? It's good to be in alphabetical order. -- To view, visit https://asterix-gerrit.ics.uci.edu/930 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6195149940f150250a65f2515e9ac9d6de2a33f9 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: Yingyi Bu <ying...@google.com> Gerrit-HasComments: Yes
