github-actions[bot] commented on code in PR #61682:
URL: https://github.com/apache/doris/pull/61682#discussion_r2985627761
##########
be/src/core/data_type_serde/data_type_date_or_datetime_serde.cpp:
##########
@@ -550,9 +571,12 @@ Status DataTypeDateSerDe<T>::from_decimal_batch(
CastParameters params {.status = Status::OK(), .is_strict = false};
for (size_t i = 0; i < decimal_col.size(); ++i) {
CppType val;
- if (CastToDateOrDatetime::from_decimal<true, IsDatetime>(
- decimal_col.get_intergral_part(i),
decimal_col.get_fractional_part(i),
- decimal_col.get_scale(), val, params)) [[likely]] {
+ if (CastToDateOrDatetime::from_decimal < DatelikeParseMode::STRICT,
Review Comment:
**Inconsistency**: This `from_decimal_batch` (non-strict path,
`params.is_strict = false`) uses `DatelikeParseMode::STRICT`, while the
equivalent functions in `data_type_datev2_serde.cpp:421` and
`data_type_datetimev2_serde.cpp:272` use `DatelikeParseMode::NON_STRICT`.
The old code in all three files used `<true>` (strict) in the non-strict
batch, so for v1 date/datetime the old behavior is preserved. However, the v2
serde files were intentionally changed to `NON_STRICT` to match the semantic
intent (`params.is_strict = false`). This v1 file should likely follow the same
correction for consistency.
The practical impact is minor — in the non-strict batch path,
`params.status` is not checked (only the return value determines nullability),
so using STRICT just does unnecessary work writing an error message to
`params.status` that nobody reads. But for consistency and clarity, this should
be `DatelikeParseMode::NON_STRICT` to match the other two serde files.
Suggested fix:
```cpp
if (CastToDateOrDatetime::from_decimal < DatelikeParseMode::NON_STRICT,
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]