Jianfeng Jia has posted comments on this change. Change subject: Fulltext search initial implementation ......................................................................
Patch Set 5: (18 comments) https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AbstractIntroduceAccessMethodRule.java: Line 298: // Full-text search consideration: an (un)ordered list of string type can be compatible with string duplicate comments? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/AccessMethodUtils.java: Line 216: // Checks whether a proper constant expression is in place for the full-text search. this comment doesn't give more information than the function name, better to remove it. https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexAccessMethod.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/InvertedIndexAccessMethod.java: Line 1183: // For full-text search can't understand the comment. better to remove it? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/metadata/queries/basic/dataset_with_meta-1/dataset_with_meta-1.1.ddl.aql File asterixdb/asterix-app/src/test/resources/metadata/queries/basic/dataset_with_meta-1/dataset_with_meta-1.1.ddl.aql: Line 30: 'text': string just want to confirm that the single quote will work? i was thinking it only works for double quote. https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/ngram-contains-panic.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/ngram-contains-panic.aql: Line 45: where string-contains($o.title, "Mu") while `string-contains` check the index? There is an ngram_idx built on this field. https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/ngram-contains.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/ngram-contains.aql: Line 44: where string-contains($o.title, "Multimedia") will it check the inverted index? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/word-contains.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-basic/word-contains.aql: Line 45: where string-contains($o.title, "Multimedia") will it check the inverted index? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-join/ngram-contains.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/inverted-index-join/ngram-contains.aql: Line 45: where string-contains($o1.title, $o2.title) and $o1.id < $o2.id will it check the inverted index? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/multi-indexes/btree-rtree-ngram-intersect.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/multi-indexes/btree-rtree-ngram-intersect.aql: Line 51: and string-contains($t.message, $keyword) will it check the inverted index? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/optimizerts/queries/multi-indexes/with-primary-index-intersect.aql File asterixdb/asterix-app/src/test/resources/optimizerts/queries/multi-indexes/with-primary-index-intersect.aql: Line 51: and string-contains($t.message, $keyword) will it check the inverted index? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-app/src/test/resources/runtimets/queries/index-selection/intersection/tinysocial-intersect.3.query.aql File asterixdb/asterix-app/src/test/resources/runtimets/queries/index-selection/intersection/tinysocial-intersect.3.query.aql: Line 27: and string-contains($t.message-text, $keyword) this test is supposed to trigger the inverted index search, so `string-contains` won't serve the purpose https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/Expression.java File asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/Expression.java: Line 35: FTCONTAINS_EXPRESSION, why full text search is an expression than just a function? https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/AbstractComparisonEvaluator.java: Line 55: protected boolean isFullTextSearchQuery = false; code explain the comment. remove the comment? Line 170: // Full-text search field (document) should be a string type. the code implies the comment https://asterix-gerrit.ics.uci.edu/#/c/989/5/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/ComparisonHelper.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/comparisons/ComparisonHelper.java: Line 491: private int fullTextContainsWithArg(ATypeTag typeTag2, IPointable arg1, IPointable[] args2) break this giant function into smaller ones? https://asterix-gerrit.ics.uci.edu/#/c/989/5/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/functions/AlgebricksBuiltinFunctions.java File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/functions/AlgebricksBuiltinFunctions.java: Line 32: FULLTEXT_CONTAINS why `FullText_Contains` is so different from the string-contains? or other functions? https://asterix-gerrit.ics.uci.edu/#/c/989/5/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/BinaryHashSet.java: Line 42: public class BinaryHashSet { where is this data structure used? What's the different from the SerialiableHashTable: https://github.com/apache/asterixdb/blob/bce1888eebdda3e306203d68c0c769a71dae739e/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java Can it be constructed from the existing SerialiableHashTable? https://asterix-gerrit.ics.uci.edu/#/c/989/5/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java: Line 142: } do we still have else ? -- To view, visit https://asterix-gerrit.ics.uci.edu/989 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I71887c2ea847e4488f4c98a11f8a5bcad02cac5a Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <[email protected]> Gerrit-Reviewer: Heri Ramampiaro <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
