Query refactoring: IdsQuery#10670
Query refactoring: IdsQuery#10670cbuescher wants to merge 5 commits intoelastic:feature/query-refactoringfrom
Conversation
There was a problem hiding this comment.
comment about visibility too, why not private with getters?
7eacb55 to
6551fe7
Compare
|
I rebased this PR on current state of feature branch, added the changes to BaseQuery and the QueryBuilder interface that we already discussed in #10669. |
There was a problem hiding this comment.
can we init the arraylist providing its size as input since we know it? also maybe use List on the left side?
|
I quickly went through it and it LGTM besides the few minor comments I left. @dakrone can you have another look too please? |
6551fe7 to
a00966b
Compare
|
Added changes adressing the comments and rebased on current feature branch. Will wait for another look from @dakrone. |
a00966b to
81e1ca9
Compare
There was a problem hiding this comment.
you dont need the type here new ArrayList<>() right? your IDE should give you a warning for this type of things.
There was a problem hiding this comment.
Actually I get a compile error when I don't use the type here. Maybe because of the ternary operator here.
|
I just had another look, left a few comments |
b749dbc to
475c083
Compare
|
Went over the comments and added serialization of string lists to the StreamInput/Output classes, also added test for checking exception on missing value field. |
475c083 to
73044ac
Compare
There was a problem hiding this comment.
can you prepend the name of the method with test, just to make sure?
297dab3 to
f4df5a0
Compare
|
Just rebased this on current state of feature branch which now includes the addition if streaming string lists & adressed the last two comments. |
|
LGTM |
Split the parse(QueryParseContext ctx) method into a parsing and a query building part, adding Streamable for serialization and hashCode(), equals() for better testing. Add basic unit test for Builder and Parser. Closes #10670
Split the parse(QueryParseContext ctx) method into a parsing and a query building part, adding Streamable for serialization and hashCode(), equals() for better testing. Add basic unit test for Builder and Parser. Closes elastic#10670
Split the parse(QueryParseContext ctx) method into a parsing and a query building part, adding Streamable for serialization and hashCode(), equals() for better testing.
Add basic unit test for Builder and Parser.
PR goes agains query-refacoring feature branch.