github-actions[bot] commented on code in PR #67697:
URL: https://github.com/apache/doris/pull/67697#discussion_r3975509190
##########
be/src/exprs/vcast_expr.cpp:
##########
@@ -130,13 +130,14 @@ Status VCastExpr::execute_column_impl(VExprContext*
context, const Block* block,
return Status::OK();
}
-bool cast_error_code(Status& st) {
- //There may be more error codes that need to be captured by try cast in
the future.
- if (st.is<ErrorCode::INVALID_ARGUMENT>()) {
- return true;
- } else {
- return false;
- }
+bool cast_error_code(const Status& st) {
+ // Value conversion failures use INVALID_ARGUMENT (parsing, numeric/date
range checks,
+ // JSONB and complex elements) or ARITHMETIC_OVERFLOW_ERRROR (decimal
conversions).
+ // Keep execution failures such as allocation errors, invalid column
shapes and corruption
+ // visible. A value failure reported as INTERNAL_ERROR must be corrected
at its producer;
+ // accepting all INTERNAL_ERROR/RUNTIME_ERROR statuses would hide
execution defects.
+ // See exprs/function/cast/README.md for the conversion-path audit.
Review Comment:
The referenced CAST audit is not present at this head
(`be/src/exprs/function/cast/README.md` does not exist), although the head
commit says that the conversion error inventory is included. Since this
allowlist decides which failures `TRY_CAST` suppresses, please either add the
audited producer inventory or make this comment self-contained/remove the
dangling reference.
##########
be/src/exprs/vcast_expr.cpp:
##########
@@ -130,13 +130,14 @@ Status VCastExpr::execute_column_impl(VExprContext*
context, const Block* block,
return Status::OK();
}
-bool cast_error_code(Status& st) {
- //There may be more error codes that need to be captured by try cast in
the future.
- if (st.is<ErrorCode::INVALID_ARGUMENT>()) {
- return true;
- } else {
- return false;
- }
+bool cast_error_code(const Status& st) {
+ // Value conversion failures use INVALID_ARGUMENT (parsing, numeric/date
range checks,
+ // JSONB and complex elements) or ARITHMETIC_OVERFLOW_ERRROR (decimal
conversions).
+ // Keep execution failures such as allocation errors, invalid column
shapes and corruption
Review Comment:
This comment says invalid column-shape failures remain visible, but the
predicate still accepts every `INVALID_ARGUMENT`. That is reachable when a
slot/column ref's runtime block type differs from the static child type
captured when FunctionCast was built: child validation uses the runtime type,
while the captured Variant V2 wrapper uses the static type and can return
`INVALID_ARGUMENT` for the resulting physical-shape mismatch. `TRY_CAST` then
converts that broken execution contract to NULL in both the batch and every row
retry. This swallowing behavior predates this PR and is not a demonstrated
legal-input regression, but the newly added guarantee is false. Please
reclassify the reachable physical/type-state guards as `INTERNAL_ERROR` and add
a propagation test, or narrow the policy claim so it does not say these
failures stay visible.
--
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]
