github-actions[bot] commented on code in PR #66833:
URL: https://github.com/apache/doris/pull/66833#discussion_r3794778365
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -670,6 +671,8 @@ public void setUserVar(String name, LiteralExpr value) {
return Literal.of(((FloatLiteral) literalExpr).getValue());
} else if (literalExpr instanceof DecimalLiteral) {
return Literal.of(((DecimalLiteral) literalExpr).getValue());
+ } else if (literalExpr instanceof DateLiteral) {
+ return Literal.fromLegacyLiteral(literalExpr,
literalExpr.getType());
Review Comment:
[P1] Keep chained user-variable casts foldable
After this returns a `TimestampTzLiteral`, `SET @rendered = CAST(@ts AS
VARCHAR(64))` is no longer accepted. `SetUserDefinedVarOp` requires the
expression to FE-fold to a literal, but `TimestampTzLiteral.uncheckedCastTo`
handles only TIMESTAMPTZ/DATETIMEV2 and throws for string-like targets, so
folding leaves a `Cast` and reports `must be constant value`. The added SELECT
still passes because it can defer the cast to BE. Please add a
session-timezone-aware string fold (not the internal UTC `getStringValue()`)
and cover a chained SET after changing `time_zone`.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -670,6 +671,8 @@ public void setUserVar(String name, LiteralExpr value) {
return Literal.of(((FloatLiteral) literalExpr).getValue());
} else if (literalExpr instanceof DecimalLiteral) {
return Literal.of(((DecimalLiteral) literalExpr).getValue());
+ } else if (literalExpr instanceof DateLiteral) {
Review Comment:
[P1] Preserve the type across forwarded user variables
This only fixes the in-process path. A user can `SET @v = CAST(... AS
TIMESTAMPTZ(6))` on a follower and then have the query forwarded when that FE
cannot read or is configured to forward. `FEOpExecutor` sends a `TExprNode`
whose `type` is still TIMESTAMPTZ(6), but
`ConnectProcessor.getLiteralExprFromThrift` ignores that field and reparses
`node.date_literal.value` with a null type before this branch runs. The value
therefore arrives here as DATE/DATETIME(V2), so rendering loses the offset and
a DST-fold predicate is coerced to wall-clock DATETIMEV2 semantics and can
return the wrong rows. Please decode `node.type` including its TIMESTAMPTZ
scale (`Type.fromThrift` currently drops that scale), use it for
reconstruction, and add a forced-forward round-trip covering rendering and
equality.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -670,6 +671,8 @@ public void setUserVar(String name, LiteralExpr value) {
return Literal.of(((FloatLiteral) literalExpr).getValue());
} else if (literalExpr instanceof DecimalLiteral) {
return Literal.of(((DecimalLiteral) literalExpr).getValue());
+ } else if (literalExpr instanceof DateLiteral) {
+ return Literal.fromLegacyLiteral(literalExpr,
literalExpr.getType());
Review Comment:
[P1] Include TIMESTAMPTZ microseconds in literal comparison
This branch makes user variables reach FE folding as `TimestampTzLiteral`,
but that class inherits `DateLiteral.compareTo`, where the fractional
tie-breaker is applied only to `DateTimeV2Literal`. Because
`TimestampTzLiteral` is a sibling subclass and `DateTimeLiteral.getValue()`
stops at whole seconds, `SET @a = ...00.100000Z; SET @b = ...00.200000Z; SELECT
@a = @b, @a < @b` folds to `1, 0` instead of `0, 1`. Please include TIMESTAMPTZ
microseconds in the comparison and add equality/ordering coverage for two user
variables within the same second.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -670,6 +671,8 @@ public void setUserVar(String name, LiteralExpr value) {
return Literal.of(((FloatLiteral) literalExpr).getValue());
} else if (literalExpr instanceof DecimalLiteral) {
return Literal.of(((DecimalLiteral) literalExpr).getValue());
+ } else if (literalExpr instanceof DateLiteral) {
+ return Literal.fromLegacyLiteral(literalExpr,
literalExpr.getType());
Review Comment:
[P1] Preserve TIMESTAMPTZ in legacy comparison coercion
With the supported global `enable_new_type_coercion_behavior=false`, user
variables declared as TIMESTAMPTZ(3) and TIMESTAMPTZ(6) no longer share an
exact type, so comparison takes the legacy common-type path. That path falls
through to DATETIME, but `CheckCast` does not allow TIMESTAMPTZ-to-DATETIME, so
the valid comparison now fails analysis instead of comparing the instants.
Please keep TIMESTAMPTZ as the common type at the maximum scale in this
compatibility branch and add differently-scaled coverage under the legacy
setting.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -670,6 +671,8 @@ public void setUserVar(String name, LiteralExpr value) {
return Literal.of(((FloatLiteral) literalExpr).getValue());
} else if (literalExpr instanceof DecimalLiteral) {
return Literal.of(((DecimalLiteral) literalExpr).getValue());
+ } else if (literalExpr instanceof DateLiteral) {
+ return Literal.fromLegacyLiteral(literalExpr,
literalExpr.getType());
Review Comment:
[P1] Preserve TIMESTAMPTZ in legacy result coercion
The legacy common-result path is separate from comparison coercion and is
also exposed by returning typed values here. With
`enable_new_type_coercion_behavior=false`, TIMESTAMPTZ(3) and TIMESTAMPTZ(6)
arguments to `NULLIF`, `IF`, or `CASE` reach
`findCommonPrimitiveTypeForCaseWhen`, which selects DATETIMEV2 instead of a
wider TIMESTAMPTZ. The allowed casts convert each instant to the current
session wall clock; across the New York fall-back, 05:05Z and 06:05Z both
become 01:05, so `NULLIF(@a,@b)` can return NULL for distinct instants. Please
preserve TIMESTAMPTZ at the maximum scale in this helper and add unequal-scale
legacy-mode `NULLIF`/CASE coverage across the fold.
--
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]
