Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.doris.qe;

import org.apache.doris.analysis.BoolLiteral;
import org.apache.doris.analysis.DateLiteral;
import org.apache.doris.analysis.DecimalLiteral;
import org.apache.doris.analysis.FloatLiteral;
import org.apache.doris.analysis.IntLiteral;
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

return Literal.fromLegacyLiteral(literalExpr, literalExpr.getType());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

} else if (literalExpr instanceof StringLiteral) {
return Literal.of(((StringLiteral) literalExpr).getValue());
} else if (literalExpr instanceof NullLiteral) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@

package org.apache.doris.nereids.rules.analysis;

import org.apache.doris.analysis.DateLiteral;
import org.apache.doris.analysis.IntLiteral;
import org.apache.doris.analysis.LargeIntLiteral;
import org.apache.doris.catalog.ScalarType;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.literal.TimestampTzLiteral;
import org.apache.doris.nereids.types.BigIntType;
import org.apache.doris.nereids.types.IntegerType;
import org.apache.doris.nereids.types.LargeIntType;
import org.apache.doris.nereids.types.SmallIntType;
import org.apache.doris.nereids.types.TimeStampTzType;
import org.apache.doris.nereids.types.TinyIntType;
import org.apache.doris.nereids.util.MemoTestUtils;
import org.apache.doris.qe.ConnectContext;
Expand Down Expand Up @@ -53,4 +58,16 @@ public void testUserVarIntegerType() {
Assertions.assertEquals(BigIntType.INSTANCE, ConnectContext.get().getLiteralForUserVar("d").getDataType());
Assertions.assertEquals(LargeIntType.INSTANCE, ConnectContext.get().getLiteralForUserVar("e").getDataType());
}

@Test
public void testUserVarTimestampTzType() {
ConnectContext ctx = MemoTestUtils.createConnectContext();
ctx.setUserVar("ts", new DateLiteral(
2024, 11, 3, 5, 5, 0, 123456, ScalarType.createTimeStampTzType(6)));

Literal literal = ctx.getLiteralForUserVar("ts");
Assertions.assertInstanceOf(TimestampTzLiteral.class, literal);
Assertions.assertEquals(TimeStampTzType.of(6), literal.getDataType());
Assertions.assertEquals("2024-11-03 05:05:00.123456+00:00", literal.getStringValue());
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !user_var_render --
2024-11-03 01:05:00.000000-04:00

-- !user_var_comparison --
1 pre_fold_utc
3 pre_explicit

-- !sql --
1 pre_fold_utc 2024-11-03 01:05:00.000000-04:00
2 post_fold_utc 2024-11-03 01:05:00.000000-05:00
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ suite("test_timestamptz_dst_fold") {
(4, 'post_explicit', CAST('2024-11-03 01:05:00 -05:00' AS TIMESTAMPTZ(6)));
"""

sql "SET time_zone = '+00:00';"
sql "SET @dst_fold_ts = CAST('2024-11-03 05:05:00 +00:00' AS TIMESTAMPTZ(6));"
sql "SET time_zone = 'America/New_York';"
qt_user_var_render "SELECT CAST(@dst_fold_ts AS VARCHAR(64));"
order_qt_user_var_comparison """
SELECT id, label
FROM tz_dst_fold_events
WHERE ts = @dst_fold_ts
ORDER BY id;
"""

sql "SET debug_skip_fold_constant = true;"
qt_sql """
SELECT id, label, CAST(ts AS VARCHAR(64)) AS rendered
Expand Down
Loading