pearu commented on PR #50146: URL: https://github.com/apache/arrow/pull/50146#issuecomment-4668734736
Two out-of-scope discoveries made while working on this, recorded here rather than folded into the PR to keep it minimal: 1. `MultipleParsersTimestampValueDecoder::Decode` (pre-existing, `csv/converter.cc`) declares its `zone_offset_present` flag once outside the parser loop. The built-in parsers happen to write the out-parameter on every call (the strptime parser unconditionally, the ISO-8601 parser resets it to `false` before scanning), so this is currently harmless — but `TimestampParser` is a public interface, and a user-implemented parser that only writes the flag when an offset is found could observe a stale value from a previous loop iteration. The new decoder in this PR declares the flag per-iteration; the timestamp decoder could get the same two-line treatment as a MINOR follow-up. 2. The "Timestamp inference/parsing" section of the C++ CSV user guide (`docs/source/cpp/csv.rst`) does not mention `ConvertOptions::timestamp_parsers` at all — custom timestamp parsing was undocumented in the user guide before the date/time subsection added here. A short paragraph there could be a docs follow-up. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
