opensearch-project / sql

Query your data using familiar SQL or intuitive Piped Processing Language (PPL)
https://opensearch.org/docs/latest/search-plugins/sql/index/
Apache License 2.0
120 stars 139 forks source link

[BUG]Should we use `Locale.ENGLISH` for monthName/dayName function #2844

Open bugmakerrrrrr opened 3 months ago

bugmakerrrrrr commented 3 months ago

What is the bug? The monthName/dayName function uses Locale.getDefault to get the display name, which may return different results in different env and cause a test error.

org.opensearch.sql.sql.DateTimeFunctionIT > testMonthName FAILED
    java.lang.AssertionError: 
    Expected: iterable with items [[September]] in any order
         but: not matched: <["九月"]>
        at __randomizedtesting.SeedInfo.seed([3027A175091234F:5336A188EBBE09E2]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at org.opensearch.sql.util.MatcherUtils.verify(MatcherUtils.java:173)
        at org.opensearch.sql.util.MatcherUtils.verifyDataRows(MatcherUtils.java:149)
        at org.opensearch.sql.sql.DateTimeFunctionIT.testMonthName(DateTimeFunctionIT.java:801)

Should we change the locale to Locale.ENGLISH?

BTW, I found that the locale of from_unixtime function is Locale.ENGLISH.

bugmakerrrrrr commented 3 months ago

@LantaoJin friendly ping:)

LantaoJin commented 3 months ago

Thanks for reporting this @bugmakerrrrrr . MonthName and DayName are MySQL-like functions. I found this https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_lc_time_names. So in my opinion, it's not a bug as I said in https://github.com/opensearch-project/sql/pull/2685#issuecomment-2236908496. But I am not quite sure that should we continue to follow all MySQL principles. cc @dai-chen @penghuo.

bugmakerrrrrr commented 3 months ago

Thanks for reporting this @bugmakerrrrrr . MonthName and DayName are MySQL-like functions. I found this https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_lc_time_names. So in my opinion, it's not a bug as I said in #2685 (comment). But I am not quite sure that should we continue to follow all MySQL principles. cc @dai-chen @penghuo.

Thanks for you context. I found that the MySQL document said that the default value is 'en_US' regardless of your system's locale setting, so if we need to follow the MySQL principle, should we change the locale to english?

LantaoJin commented 3 months ago

so if we need to follow the MySQL principle, should we change the locale to english?

That's what I am not quite sure. OpenSearch SQL doesn't provide a system function/variable to set a lc_time_name, if we change it to Locale.ENGLISH, it will restrict to English.