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.

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

IGNITE-14699 Add IndexQuery API. #9118

Merged
merged 27 commits into from Sep 17, 2021

Conversation

timoninmaxim
Copy link
Member

@timoninmaxim timoninmaxim commented May 21, 2021

@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch from a0c542f to 659b920 Compare May 21, 2021 18:37
@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch from 8a20778 to 93d4ec2 Compare June 10, 2021 19:00
for (int i = 0; i < rngCond.fields.size(); i++) {
String f = rngCond.fields.get(i);

A.ensure(!fields.contains(f), "Duplicated field in conditions: " + f + ".");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we use the same field twice? For example, if we proceed data month by month a condition would be something like: d >= 01/01/2021 and d < 01/02/2021 and then next query: d >= 01/02/2021 and d < 01/03/2021, etc. With the current implementation, we can't build such conditions (gte and lt on the same field).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's better to add new method with inclusion flags: between(field, lower, upper, lowIncl, upIncl). Otherwise logic could be too hard - how to merge lt and lt, or lt and lte). WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed privately, decided to provide opportunity to define multiple conditions on the same field and merge them with AND. Will be done in separate ticket.

@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch from 829e857 to dd5e285 Compare June 17, 2021 16:23
@alex-plekhanov
Copy link
Contributor

Let's also add some test where the index field alias not equal to java field name.

@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch from bc1f6f6 to fc748c5 Compare July 1, 2021 08:21
@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch 2 times, most recently from d3850b0 to 2d66613 Compare July 1, 2021 08:52
@timoninmaxim timoninmaxim force-pushed the IGNITE-14699__index_queries_api branch from 2d66613 to 76807f2 Compare July 1, 2021 08:54
… replace Null class with flags, PriorityQueue for cursors, refactoring.
@timoninmaxim
Copy link
Member Author

@AMashenkov I've pushed fixes, could you please have a look?

@timoninmaxim
Copy link
Member Author

@AMashenkov hi! I fixed merge conflict. Now, this PR is available for review again.

@AMashenkov AMashenkov merged commit 9cbc585 into apache:master Sep 17, 2021
@timoninmaxim timoninmaxim deleted the IGNITE-14699__index_queries_api branch September 17, 2021 11:12
xintrian pushed a commit to xintrian/ignite that referenced this pull request May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants