Repository navigation
[C++][Python] strptime fails to parse with %p on Windows #31971
Description
Activity
Antoine Pitrou / @pitrou:
As I said on the Linked GH issue:The vendored strptime implementation does support %p, but we have to disable it on Windows because the behavior is locale-dependent and it's not easy to support there.
(of course, we may support it in a non-locale-dependent way, i.e. always use the same values as a "C" Unix locale)
Rok Mihevc / @rok:
Also see ARROW-16443Antoine Pitrou / @pitrou:
For the record, here are the changes we applied to the vendored strptime:
https://gist.github.com/pitrou/d1d23ad2fdae4be34e737c14c8ae7443You can see that, on Windows, we disable any locale-specific codes because they use
nl_langinfowhich is unavailable.Joris Van den Bossche / @jorisvandenbossche:
We could maybe adapt the patch to the vendored strptime to raise a more informative error if it encounters%pif that is explicitly not supported?Rok Mihevc / @rok:
That would be useful indeed. Judging by ibis we might want to check %c, %x and %X too.PR #50146 (primarily about CSV date/time parsing) includes a fix for this:
%pin the vendored muslstrptimeused on Windows was compiled out becausenl_langinfo()(the source of the AM/PM strings) is unavailable there. The PR adds static C-locale tables, after which%pcorrectly adjusts%Ihours on Windows (02:30 PM→ 14:30). Validated on Windows CI across MinGW, MSVC, and ARM64 MSVC builds.
As reported in #13111 parsing a timestamp with %p will fail on Windows. This is probably due to issues with vendored strptime on Windows locales.
We should explore which flags can be enabled and how. Strptime tests suite should be expanded https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernels/scalar_string_test.cc#L1842-L1890.
Reporter: Rok Mihevc / @rok
Watchers: Rok Mihevc / @rok
Related issues:
Note: This issue was originally created as ARROW-16618. Please see the migration documentation for further details.