Refactor of GeohashCellQuery#13393
Refactor of GeohashCellQuery#13393colings86 merged 1 commit intoelastic:feature/query-refactoringfrom colings86:refactor/geohash-cell-query
Conversation
There was a problem hiding this comment.
I think we have to decide whether ParseFields should be in query builders or query parsers. Good to share these two though given that they are always the same. They should indeed be in some base class.
|
left a few minor comments, looks good |
|
@javanna I pushed a commit which addresses your comments |
There was a problem hiding this comment.
don't we have sensible defaults for levels and neighbors?
There was a problem hiding this comment.
I'll add a default of false for neighbors but null is actually the sensible default for levels as it means 'do not truncate the given geohash'. The levels parameter is only there to truncate the precision of the given geohash if required
|
left one tiny comment around default values in the builder, LGTM besides that |
Moving the query building functionality from the parser to the builders new toQuery() method analogous to other recent query refactorings. Relates to #10217 PR goes against the query-refactoring branch
Moving the query building functionality from the parser to the builders
new toQuery() method analogous to other recent query refactorings.
Relates to #10217
PR goes against the query-refactoring branch