[CI] SimpleQueryStringBuilderTests.testToQuery failure

[droberts195]

SimpleQueryStringBuilderTests.testToQuery failed in a 7.2 build: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+7.2+matrix-java-periodic/ES_BUILD_JAVA=openjdk12,ES_RUNTIME_JAVA=zulu8,nodes=immutable&&linux&&docker/13/console

The error is:

org.elasticsearch.index.query.SimpleQueryStringBuilderTests > testToQuery FAILED
    java.lang.AssertionError: query was marked as cacheable in the context but this test indicates it should not be cacheable: {
      "simple_query_string" : {
        "query" : "NoWt",
        "flags" : -1,
        "default_operator" : "or",
        "analyze_wildcard" : false,
        "quote_field_suffix" : "",
        "auto_generate_synonyms_phrase_query" : true,
        "fuzzy_prefix_length" : 4,
        "fuzzy_max_expansions" : 5,
        "fuzzy_transpositions" : true,
        "boost" : 1.0
      }
    }

This reliably reproduces on the 7.2 branch using this command:

./gradlew :server:test --tests "org.elasticsearch.index.query.SimpleQueryStringBuilderTests.testToQuery"   -Dtests.seed=9492DAEC1EE2893   -Dtests.security.manager=true   -Dtests.locale=lv-LV   -Dtests.timezone=Europe/Dublin   -Dcompiler.java=12   -Druntime.java=8

So it seems like a seed-specific failure.

[elasticmachine]

Pinging @elastic/es-search

[cbuescher]

Note: reproduces on 7.2 but not on 7.x for me with the above seed.

[cbuescher]

The problem is an edge case where the above query is marked as cachable, but out test logic says its not, most likely because of the "query" : "NoWt" part. Maybe we need to be more carful with random string generation here.

[cbuescher]

For this query in particular:

"simple_query_string" : {
        "query" : "NoWt",
        "flags" : -1,
        "default_operator" : "or",
        "analyze_wildcard" : false,
        "quote_field_suffix" : "",
        "auto_generate_synonyms_phrase_query" : true,
        "fuzzy_prefix_length" : 4,
        "fuzzy_max_expansions" : 5,
        "fuzzy_transpositions" : true,
        "boost" : 1.0
      }

the test framework assumes it should be cacheable (the check in https://github.com/elastic/elasticsearch/blob/7.2/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java#L39 lowercases the query term before checking the prefix for "now") but in reality the JavaDateMathParser (and also the legacy Joda one) only handle now (and thus flip the "cacheable" flag in the seach context) when the prefix is an exact match (no lowercasing).

I tried changing FullTextQueryTestCase to be case sensitive, but that creates other kind of problems with cases where we apply analysis to the query term before it potentially gets fed to a date field, so that doesn't work either.

Looking at this I think we push too hard on the randomization and handling all edge cases in a generic way in these tests. I think we should exclude query terms that start with "now" in whatever case variation from the general test cases and by that avoid the tricky caching detection problems and instead add dedicated tests case just for these edge cases.

[jimczi]

I tried changing FullTextQueryTestCase to be case sensitive, but that creates other kind of problems with cases where we apply analysis to the query term before it potentially gets fed to a date field, so that doesn't work either.

Do you have an example of this ? I could not think of a case where we'd modify the original term before feeding it to an analyzer.

I tried changing FullTextQueryTestCase to be case sensitive, but that creates other kind of problems with cases where we apply analysis to the query term before it potentially gets fed to a date field, so that doesn't work either.

I agree, if this turns too complicated we can ensure that the random query string doesn't start with now but I'd like to understand first why the current solution couldn't work if we don't ignore the case.

[jimczi]

We discussed offline and since we also randomize the forced analyzer that is used there is a chance that we lowercase the term. So +1 to remove the randomization to check if a query is cacheable and to make dedicated tests for it on all full-text queries.