Support mvn build and integration-test for Elasticsearch 7.0.1 #51
Conversation
src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java
Show resolved
Hide resolved
| @@ -86,7 +88,7 @@ public void greaterThan() { | |||
| @Test | |||
| public void greaterThanOrEqualTo() { | |||
| assertThat( | |||
| query(SELECT_FROM + "WHERE date_format(insert_time, 'YYYY-MM-dd', 'UTC') >= '2014-08-23' LIMIT 1000"), | |||
| query(SELECT_FROM + "WHERE date_format(insert_time, 'yyyy-MM-dd', 'UTC') >= '2014-08-23' LIMIT 2000"), | |||
There was a problem hiding this comment.
Why do you increase the limits ?
There was a problem hiding this comment.
The return hits in ES 7.0 is different with ES 6.7. ES 7.0 return value doesn't include "2014-08-24" is limited 1000.
There was a problem hiding this comment.
But is it the expected behavior? I am ok with updating the test queries as long as we know the test is checking the right thing.
There was a problem hiding this comment.
Yes, it is the expected behavior. In the test data set, there are 1424 records has insert_time 2014-08-23. With the limit 1000, in ES 6.7, the return hits include record with insert_time as 2014-08-23 and 2014-08-24. But in ES 7.0, with the limit 1000, the result only include insert_time 2014-08-23. I didn't find the doc which mention it, but it seems the ES 7.0 sort the output in order.
| "OR date_format(insert_time, 'YYYY-MM-dd', 'UTC') > '2014-08-23'"), | ||
| "WHERE date_format(insert_time, 'yyyy-MM-dd', 'UTC') < '2014-08-18' " + | ||
| "OR date_format(insert_time, 'yyyy-MM-dd', 'UTC') > '2014-08-23'" + | ||
| "ORDER BY insert_time " + |
There was a problem hiding this comment.
Why are you modifying the query (order by) ?
There was a problem hiding this comment.
+1. There is a "containsInAnyOrder" below, not sure why "order by" here.
There was a problem hiding this comment.
not necessory, "ORDER BY insert_time" is copied from other integration test. will remove it.
| @@ -6,13 +6,13 @@ | |||
| {"messageTile": "c", "join_field": "parentType"} | |||
| {"index":{"_type": "joinType", "_id":"4"}} | |||
| {"messageTile": "d", "join_field": "parentType"} | |||
| {"index":{"_type": "joinType", "_id":"5", "_routing": "1"}} | |||
| {"index":{"_type": "joinType", "_id":"5", "routing": "1"}} | |||
There was a problem hiding this comment.
Why renaming _routing to routing?
There was a problem hiding this comment.
This is the breaking changes in ES 7.0. The _routing with underscore has been removed.
| @@ -257,7 +257,7 @@ public void functionLogs() throws Exception { | |||
| @Test | |||
| public void functionPow() throws Exception { | |||
| String query = "SELECT pow(account_number, 2) as key,"+ | |||
| "abs(age - 60) as new_age from " + TEST_INDEX_ACCOUNT + "/account limit 1"; | |||
| "abs(age - 60) as new_age from " + TEST_INDEX_ACCOUNT + "/account WHERE firstname = 'Virginia' and lastname='Ayala' limit 1"; | |||
There was a problem hiding this comment.
With the same query, the return hit in ES 7.0 is different with ES 6.7. So I add firstname and lastname to make sure the same hit will be returned.
In case, the test is checking the function of pow. it should be fine.
| containsInAnyOrder("2014-08-17") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void lessThanOrEqualTo() { | ||
| assertThat( | ||
| query(SELECT_FROM + "WHERE date_format(insert_time, 'YYYY-MM-dd', 'UTC') <= '2014-08-18'"), | ||
| query(SELECT_FROM + "WHERE date_format(insert_time, 'yyyy-MM-dd', 'UTC') <= '2014-08-18' " + | ||
| "ORDER BY insert_time " + |
There was a problem hiding this comment.
Another "order by" should be removed.
*Issue #, if available: N/A
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.