924060929 commented on code in PR #64366:
URL: https://github.com/apache/doris/pull/64366#discussion_r3569169898
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/SelectHintSetVar.java:
##########
@@ -66,6 +66,7 @@ public void setVarOnceInSql(StatementContext context) {
}
}
}
+ context.refreshStatementTimeZone();
Review Comment:
`refreshStatementTimeZone()` is outside the `try { VariableMgr.setVar(...) }
catch (Throwable t) { throw new AnalysisException(...) }` block above it.
Because `time_zone` has no `@VarAttr` checker, `VariableMgr.setVar` accepts an
invalid/non-canonical hint value without validation, and this unguarded
`refreshStatementTimeZone()` then does `ZoneId.of(rawValue, aliasMap)` — so an
invalid `SET_VAR(time_zone=...)` surfaces as a **raw
`java.time.DateTimeException`** rather than the clean `AnalysisException("Can
not set session variable 'time_zone' = ...")` this method produces for every
other bad var.
Verified against this head (unit test):
```
refreshStatementTimeZone('8:00') threw:
java.time.DateTimeException: Invalid ID for region-based ZoneId, invalid
format: 8:00
```
It's also a slightly broadened throw surface vs before this PR:
`refreshStatementTimeZone()` runs for every `SET_VAR` hint during analysis, so
a query that merely carries an invalid time_zone hint but uses **no** time
function now errors, whereas pre-PR it only threw if a `now()`/`curdate()` was
actually folded (confirmed on a cluster: pre-PR `SELECT /*+
SET_VAR(time_zone='8:00') */ 1` returns `1`, while `... now()` already threw
the same raw exception). Valid zones and all `SET time_zone=...` statement
values are unaffected — so this is invalid-input/error-quality only, not a
regression for well-formed queries.
Suggest either moving `refreshStatementTimeZone()` inside the `try` (so it
yields the same `AnalysisException`), or validating the hint's `time_zone` via
`TimeUtils.checkTimeZoneValidAndStandardize(...)` before storing it.
--
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]
