Skip to content

ES6 compatible queries#1354

Merged
orangejulius merged 4 commits intomasterfrom
es6-queries
Oct 8, 2019
Merged

ES6 compatible queries#1354
orangejulius merged 4 commits intomasterfrom
es6-queries

Conversation

@orangejulius
Copy link
Member

@orangejulius orangejulius commented Oct 1, 2019

This PR uses the work in pelias/query#109 to convert our Elasticsearch queries into an ES6 compatible format.

The main change is that we can no longer set the phrase property on match queries. Phrase queries must now use the match_phrase query which makes the distinction between the two query types much more clear

Note: this PR is currently based on top of #1356, which should be merged first.

Closes #1261
Connects pelias/pelias#719

@orangejulius orangejulius force-pushed the es6-queries branch 3 times, most recently from c0876a3 to 954f779 Compare October 1, 2019 18:38
@orangejulius
Copy link
Member Author

orangejulius commented Oct 2, 2019

There are a bunch of acceptance test failures on this branch. Its possible that the previous queries were not being interpreted by Elasticsearch as match_phrase queries. Some investigation will be needed to decide how to proceed. We can turn all the match_phrase queries into match queries if we need to though.

@orangejulius orangejulius force-pushed the es6-queries branch 2 times, most recently from c7fee9a to ac7ba7d Compare October 3, 2019 22:18
@orangejulius
Copy link
Member Author

orangejulius commented Oct 3, 2019

Turns out the error was due to a subtle mistake while converting the queries to ES6:

vs.var('input:name').set( tokens.join(' ') );

The input:name variable was being overridden due to a line that should have been removed. This actually didn't have any effect on the query, but did cause the max_character_count_layer_filter to exclude addresses since it believed the input text was only 3 characters. I'm struggling to figure out how to put a unit test in place to test for this scenario without creating an entire new fixture.

But the good news is that this code is now good to go!

@orangejulius orangejulius marked this pull request as ready for review October 4, 2019 19:34
Copy link
Member

@missinglink missinglink left a comment

Choose a reason for hiding this comment

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

yea looks good to me

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.

Phrase queries in Elasticsearch 6

2 participants