SQL: Ignore H2 comparative tests for uppercasing/lowercasing string functions #32604
Conversation
… (which considers the Locale). ES-SQL is, so far, ignoring the Locale. Still, the same queries are executed against ES-SQL alone and results asserted to be correct.
|
Pinging @elastic/es-search-aggs |
| // selectInsertWithLcaseAndLengthWithOrderBy | ||
| //SELECT "first_name" origFN, "last_name" origLN, INSERT(UCASE("first_name"),LENGTH("first_name")+1,123,LCASE("last_name")) modified FROM "test_emp" WHERE ASCII("first_name")=65 ORDER BY "first_name" ASC, "last_name" ASC LIMIT 10; | ||
| // SELECT "first_name" origFN, "last_name" origLN, INSERT(UCASE("first_name"),LENGTH("first_name")+1,123,LCASE("last_name")) modified FROM "test_emp" WHERE ASCII("first_name")=65 ORDER BY "first_name" ASC, "last_name" ASC LIMIT 10; |
There was a problem hiding this comment.
I think instead of commenting them out we could move them into a separate sql-spec file and we can add a logic that will not add them to the list of test here for locales where H2 is messed up.
There was a problem hiding this comment.
Yep. Makes sense. I'll make the change.
| @@ -0,0 +1,28 @@ | |||
| //https://github.com/elastic/elasticsearch/issues/31863 | |||
There was a problem hiding this comment.
I think the file should be case_functions.sql-spec or something and we should include this file in the test case only if we're in a locale the h2 is fine with.
There was a problem hiding this comment.
Or, better, we should include them in the test but skip them. I think that is better from a reporting standpoint.
There was a problem hiding this comment.
@nik9000 can we use rest blacklist to blacklist them? I think having something like this only with local check would be nice.
Alternatively, something like
if (goodLocale) {
tests.addAll(readScriptSpec("/unsupported.sql-spec", parser));
}
would work as well, it's just I agree with your point about reporting.
There was a problem hiding this comment.
I think something like
if (name.startsWith(unsupported)) {
assumeTrue(goodLocale);
}
Would be better. I don't like changing the tests we run based on the locale but I don't mind skipping them based on locale.
|
test this please |
…unctions (#32604) Skip the comparative tests using lowercasing/uppercasing against H2 (which considers the Locale). ES-SQL is, so far, ignoring the Locale. Still, the same queries are executed against ES-SQL alone and results asserted to be correct.
ES-SQL is, so far, ignoring the Locale, but we have comparative tests with H2 that are failing because of this.
Still, the same queries are executed against ES-SQL alone and results asserted to be correct.
We should investigate if a Locale should be used for some functions: #32603
Fixes #32589