SQL: day and month names tests workaround #33653
Conversation
…atting for day and month names.
|
Pinging @elastic/es-search-aggs |
|
We discuss with Andrei it would be good to enforce using
|
There was a problem hiding this comment.
I think the locale is the canary in the mine.
- Is the
COMPATlocale used by ES itself when running (like we do for other JVM options)?
If it is then it's a bug in the way the JVMs are started. If not, then it's something that needs to be fixed in the SQL functions code as the functions need to result the same result regardless of the JVM used underneath.
Further more considering this is the default behavior in Java 9, it is likely the COMPAT flag is on its way out.
I would assume there's a formatting option that's available in Java9 (which can be detected while keeping the Java 8 compatibility in place) to get a hold of the short format - if not then returning the first 3 chars should work.
|
@costin yes - ES enforces this specific value for |
… to unit tests as it should
| } | ||
| String beforeJava9CompatibleLocale = System.getProperty("java.locale.providers"); | ||
| // and COMPAT setting needs to be first on the list | ||
| boolean isBeforeJava9Compatible = beforeJava9CompatibleLocale != null && beforeJava9CompatibleLocale.split(",")[0].equals("COMPAT"); |
There was a problem hiding this comment.
What if it's not the first in the list? This piece can be more reliable with something like:
Arrays.toList(Strings.tokenizeToStringArray(beforeJava9...)).contains("COMPAT") - works with whitespace, nulls, out-of-order arguments.
There was a problem hiding this comment.
My understanding is that the list is a preference list with the first provider being the first one checked and used. But I see your point about a more reliable check.
There was a problem hiding this comment.
Even If it's the case that needs to be first, I agree it would be nice to use the tokenize to trim whitespaces.
…lastic#33653) DAYNAME and MONTHNAME functions tests will be skipped if the right JVM parameter (-Djava.locale.providers=COMPAT) is not used in unit testing environment
* master: (46 commits) Fixing assertions in integration test (elastic#33833) [CCR] Rename idle_shard_retry_delay to poll_timout in auto follow patterns (elastic#33821) HLRC: Delete ML calendar (elastic#33775) Move DocsStats into Engine (elastic#33835) [Docs] Clarify accessing Date methods in painless (elastic#33560) add elasticsearch-shard tool (elastic#32281) Cut over to unwrap segment reader (elastic#33843) SQL: Fix issue with options for QUERY() and MATCH(). (elastic#33828) Emphasize that filesystem-level backups don't work (elastic#33102) Use the global doc id to generate a random score (elastic#33599) Add minimal sanity checks to custom/scripted similarities. (elastic#33564) Profiler: Don’t profile NEXTDOC for ConstantScoreQuery. (elastic#33196) [CCR] Change FollowIndexAction.Request class to be more user friendly (elastic#33810) SQL: day and month name functions tests locale providers enforcement (elastic#33653) TESTS: Set SO_LINGER = 0 for MockNioTransport (elastic#32560) Test: Relax jarhell gradle test (elastic#33787) [CCR] Fail with a descriptive error if leader index does not exist (elastic#33797) Add ES version 6.4.2 (elastic#33831) MINOR: Remove Some Dead Code in Scripting (elastic#33800) Ensure realtime `_get` and `_termvectors` don't run on the network thread (elastic#33814) ...
Because of #33621 (comment) and the fact that
-Djava.locale.providers=COMPAToption is not passed along when the jvm is forked for a test, the tests use a different approach to verify the DAYNAME and MONTHNAME functions.Fixes #33621