ianmcook commented on code in PR #50146:
URL: https://github.com/apache/arrow/pull/50146#discussion_r4251625484
##########
cpp/src/arrow/csv/converter.cc:
##########
@@ -464,18 +474,156 @@ struct MultipleParsersTimestampValueDecoder : public
ValueDecoder {
}
protected:
- using ParserVector = std::vector<const TimestampParser*>;
+ TimeUnit::type unit_;
+ bool expect_timezone_;
+ std::vector<const TimestampParser*> parsers_;
+};
+
+//
+// Value decoder for dates and times, with fallback to user-defined
+// timestamp parsers
+//
+
+// Splits the fractional seconds (up to 9 digits) off an ISO-8601 timestamp
+// "YYYY-MM-DD[ T]hh:mm:ss.s{1,9}" with an optional zone offset. Returns `s`
+// if there are no fractional seconds, a copy without them in `buffer`, or
+// nullptr if they are malformed. The split-off digits are returned in
+// `subseconds`.
+const char* SplitISO8601Subseconds(const char* s, uint32_t* size, char
(&buffer)[64],
+ std::string_view* subseconds) {
+ if (*size <= 20 || s[19] != '.') {
+ return s;
+ }
+ uint32_t fraction_length = 0;
+ while (20 + fraction_length < *size && s[20 + fraction_length] >= '0' &&
+ s[20 + fraction_length] <= '9') {
+ ++fraction_length;
+ }
+ const uint32_t rest_length = *size - 20 - fraction_length;
+ if (fraction_length == 0 || fraction_length > 9 || 19 + rest_length >
sizeof(buffer)) {
+ return nullptr;
+ }
+ std::memcpy(buffer, s, 19);
+ std::memcpy(buffer + 19, s + 20 + fraction_length, rest_length);
Review Comment:
Could we validate the suffix after the fractional digits before copying it
into `buffer`? The new splitting logic accepts timestamps with two fractional
components.
For an explicit `time32[ms]` column with `timestamp_parsers=[ISO8601]`:
- `2020-03-15 14:30:00.5.5` incorrectly succeeds as `14:30:01.000`.
- `2020-03-15 23:59:59.9.9` produces `86400800` milliseconds, outside the
valid daily range, so the resulting table fails `ValidateFull()`.
The splitter removes the first fraction, leaving the second for the ISO
parser to consume, and then the decoder adds the first fraction back. This also
affects `time64[us]` and `time64[ns]`.
Please ensure the suffix is empty or a valid zone offset before
reconstructing the string, and add regression tests rejecting both examples.
--
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]
