Fixed DateTimeFunctionTest.testWeekOfYearWithTimeType and YearWeekTestt.testYearWeekWithTimeType Test Failures

[kenrickyap]

Description

Certain tests failed due to inconsistencies with how ISO 8601 and how opensearch sql calculates number of weeks since start of year. Specifically ISO 8601 calculates weeks using the following criteria:

• Weeks start on Monday
• The first week of a year is any week containing 4 or more days in the new year.

Whereas YEARWEEK counts only full weeks, where weeks start on Sunday. To fix the discrepancy we find the first Sunday of the year and start counting weeks from that date.

Related Issues

Resolves #2477

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[LantaoJin]

@kenrickyap could this patch address the flaky test listed in and workaround by PR #3225? If yes, please update that test too.

[kenrickyap]

@kenrickyap could this patch address the flaky test listed in and workaround by PR #3225? If yes, please update that test too.

@LantaoJin I have tried to apply the same logic but for some reason the unit test is still failing after I remove the check for January and December. Will spend a little more time to find a solution!

[LantaoJin]

#3235 (comment)

any update?

[kenrickyap]

#3235 (comment)

any update?

Have found fix for #3225. However, in finding the fix have identified inconsistency of behaviour between extract and yearweek implementation. Specifically, extract is calculating week_of_year using java.time libaray (setting Local to Local.English) where as yearweek using CalendarLookup.getWeekNumber (to align with MySQL). Depending on which function users use, they will get a different value.

I believe it would be best to align the behaviour of extract and yearweek. However as the current test are failing and preventing other PRs from being merge, we should merge this ticket asap and aim to align function behaviour in another ticket

[kenrickyap]

@YANG-DB @acarbonetto @LantaoJin would we be able to review and merge this ticket? Currently the above unit tests are failing for #3230

[acarbonetto]

Have found fix for #3225. However, in finding the fix have identified inconsistency of behaviour between extract and yearweek implementation. Specifically, extract is calculating week_of_year using java.time libaray (setting Local to Local.English) where as extract using CalendarLookup.getWeekNumber (to align with MySQL). Depending on which function users use, they will get a different value.

This as a separate bug with reference to the work here. Aligning implementation for extract and extract can be out of scope for this issue and can be solved separately.

Will raise a BUG and get this moving forward as it is blocking further development.

[noCharger]

CI failed on BWC, doesn't seem related though

Execution failed for task ':integ-test:sqlBwcCluster#mixedClusterTask'.
> `cluster{:integ-test:sqlBwcCluster0}` failed to wait for cluster health yellow after 40 SECONDS
    IO error while waiting cluster
    503 Service Unavailable

[acarbonetto]

CI failed on BWC, doesn't seem related though

Execution failed for task ':integ-test:sqlBwcCluster#mixedClusterTask'.
> `cluster{:integ-test:sqlBwcCluster0}` failed to wait for cluster health yellow after 40 SECONDS
    IO error while waiting cluster
    503 Service Unavailable

No it is not. See: #3184