Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Improve]delete match_element_xx and add fe config for create inverted index on array #34766

Merged
merged 3 commits into from
May 14, 2024
Merged

[Improve]delete match_element_xx and add fe config for create inverted index on array #34766

merged 3 commits into from
May 14, 2024

Conversation

amorynan
Copy link
Contributor

Proposed changes

in this pr:

  1. delete match_element_xx grammar for array
  2. add a fe config enable_create_inverted_index_for_array array type which default is false

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

xiaokang
xiaokang reviewed May 13, 2024
View reviewed changes
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4
@@ -1168,7 +1168,7 @@ nonReserved
| MAP
| MATCH_ALL
| MATCH_ANY
| MATCH_ELEMENT_EQ
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete others

fe/fe-common/src/main/java/org/apache/doris/common/Config.java
@@ -2694,6 +2694,9 @@ public static boolean isNotCloudMode() {
@ConfField(mutable = true, masterOnly = true)
public static boolean enable_create_bitmap_index_as_inverted_index = false;

@ConfField(mutable = true)
public static boolean enable_create_inverted_index_for_array = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If array_contains is OK, keep it true and disable index def with parser

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we also need it for 2.1 because here depends on this pr: #32620 enable expr push down storage layer to pass inverted index , but this feature is not gonna to be in 2.1

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

eldenmoon
eldenmoon requested changes May 13, 2024
View reviewed changes
Copy link
Member

@eldenmoon eldenmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may cause compability issue

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@amorynan
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.66% (8983/25191)
Line Coverage: 27.32% (74273/271837)
Region Coverage: 26.56% (38389/144526)
Branch Coverage: 23.38% (19577/83738)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b167ab9194a9b4d91ffa4d548012b6d4a1f9893b_b167ab9194a9b4d91ffa4d548012b6d4a1f9893b/report/index.html

eldenmoon
eldenmoon approved these changes May 14, 2024
View reviewed changes
Copy link
Member

@eldenmoon eldenmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@eldenmoon eldenmoon merged commit 75ad0ff into apache:master May 14, 2024
23 of 28 checks passed
ByteYue pushed a commit to ByteYue/doris that referenced this pull request May 15, 2024
@amorynan @ByteYue
[Improve]delete match_element_xx and add fe config for create inverte…
f088a66 
…d index on array (apache#34766)
This was referenced Jun 4, 2024
Merged
Merged
xiaokang pushed a commit that referenced this pull request Jun 6, 2024
@amorynan
[FIX] Pick array inverted index bugfix (#35837)
b5a35b9 
here with some array with inverted index bugfix:
see also: 
#34766
#35086
#34683
#34076
xiaokang pushed a commit that referenced this pull request Jun 12, 2024
@amorynan
[FIX](array)fix array with inverted index #35837 #34766 #34683 #34076 (…
0bd451d 
…#35926)

## Proposed changes
see also for branch-2.1:  #35837
and master: 
#34766
#35086
#34683
#34076
mongo360 pushed a commit to mongo360/doris that referenced this pull request Aug 16, 2024
@amorynan
[FIX](array)fix array with inverted index apache#35837 apache#34766 a…
a5b995c 
…pache#34683 apache#34076 (apache#35926)

## Proposed changes
see also for branch-2.1:  apache#35837
and master:
apache#34766
apache#35086
apache#34683
apache#34076
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaokang xiaokang xiaokang left review comments

@eldenmoon eldenmoon eldenmoon approved these changes

@morrySnow morrySnow morrySnow approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.0.12-merged dev/2.1.4-merged dev/3.0.0-merged meta-change reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@amorynan @doris-robot @xiaokang @eldenmoon @morrySnow @dataroaring