jacktengg opened a new pull request, #70380:
URL: https://github.com/apache/doris/pull/70380
### Background
A follower FE forwards CTAS to the master FE. The request includes user
variables as typed Thrift literals. The master restores these variables before
it analyzes the statement.
### What problem does this PR solve?
Issue Number: None
Related PR: None
PR Type: Bugfix
Problem Summary:
Forwarded TIMESTAMPTZ user variables can become DATE or DATETIME values.
CTAS can then omit matching rows, create the wrong column type, or return an
incorrect time difference. Restore both the declared type and its scale during
decoding.
### Details
- **Frequency:** When a follower forwards CTAS that reads a TIMESTAMPTZ user
variable.
- **Root cause:** Literal decoding discards the TIMESTAMPTZ type. The shared
type decoder also falls back to TIMESTAMPTZ with scale 0.
- **Why existing tests missed it:** Date forwarding tests cover
TIMESTAMP_NS. TIMESTAMPTZ parser tests do not cover the complete Thrift round
trip.
- **Fix:** Decode the TIMESTAMPTZ scale from the existing type descriptor.
Pass that type to the existing literal parser. Add type and literal round-trip
tests, plus a CTAS regression with one source row.
- **Other paths and branches:** The shared type decoder also serves Java UDF
types and nested type descriptors. Its TIMESTAMPTZ branch now retains scale. A
partition snapshot comment is updated; its conversion logic does not change. No
release branch is modified.
- **Behavior change:** CTAS through a follower preserves the TIMESTAMPTZ
type, scale, and UTC value used by a direct master connection.
### Testing
- Local FE unit tests: 57 passed. Command: `bash run-fe-ut.sh --run
org.apache.doris.qe.ConnectProcessorTimestampTzTest,org.apache.doris.qe.ConnectProcessorTimestampNsTest,org.apache.doris.analysis.DateLiteralUtilsTest,org.apache.doris.catalog.TypeTest`.
- Before the fix, all four forwarded TIMESTAMPTZ cases failed on source
revision f7396d7229a. With only the literal fix, three cases still failed
because scale became 0. Both fixes make all four cases pass.
- Final FE build and Checkstyle passed: `FE_MAVEN_THREADS=8
DISABLE_BE_JAVA_EXTENSIONS=ON bash build.sh --fe`.
- Ran `bash run-regression-test.sh --run -d datatype_p0/timestamptz -s
test_timestamptz_user_variable_ctas` through both FE roles. Both passed with
the final build in an isolated two-FE, one-BE cluster.
- The SQL test uses one source row and script-generated expected results. It
verifies the matching row, TIMESTAMPTZ(6) column, +08:00 value, and zero hour
difference.
- The earlier multi-FE reproduction used existing b9829c5f871 binaries:
master passed; follower returned no filtered row, a DATE column, and an 8-hour
difference. Final FEs use the current source plus this fix. The BE uses the
existing worktree binary; BE code was not changed or rebuilt.
- `git -c core.whitespace=-blank-at-eof diff --cached --check` passed. The
generated regression output retains its standard final blank line.
### Release note
Fix CTAS results when a follower FE forwards TIMESTAMPTZ user variables.
Matching rows, time differences, column types, and fractional precision now
agree with direct master execution.
### Check List (For Author)
- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Ready
### AI Disclosure
- Tool: Codex
- Generated: Code changes, tests, and this description.
- Human review: Pending.
### Background
<!-- How the feature or module this PR touches works today, in two to four
plain sentences, for a reviewer who doesn't know it. Link the user doc if there
is one. Leave the problem and the fix to Problem Summary. -->
### What problem does this PR solve?
<!-- Issue Number: "close #N" if this PR resolves the issue, "#N" if it only
relates to it, "None" if there is none. A Feature needs an issue. -->
<!-- Related PR: "#N (relation)" for each PR a reviewer should know about,
such as "#123 (introduced the bug)", separated by commas. Write "None" if no
related PR is known. -->
<!-- PR Type: Bugfix, Performance, Feature, Enhancement, Refactor, Test,
Build/CI/Chore, Revert, Release/Dependency, or Other. -->
Issue Number:
Related PR:
PR Type:
Problem Summary:
<!-- Two to five sentences: the problem or need and who is affected; the
approach in one sentence; when the diff is larger than that suggests, what else
it had to handle, or that most of it is tests; and what existing users see
after an upgrade. -->
### Details
<!--
For a diff of more than about 300 changed lines, start with "- **Change
map:**": how many lines are production code, tests, generated files, and docs,
then the code in parts, each with the problem it handles and its size.
Then answer each field of your PR type on its own line, as "- **Field:**
answer", in this order:
Bugfix: Frequency, Root cause, Why existing tests missed it, Fix, Other
paths and branches
Performance: Why it was slow, Optimization, Results, Applicability and cost
Feature: Scope, Design, Usage, Default behavior, Compatibility
Enhancement: Before and after, Configs and defaults, Impact on existing
users
Refactor: Moves vs. logic changes, Behavior change, Compatibility,
Performance impact, Rollback
Test, adding tests: Scenarios, What existing tests miss, Fails when broken
Test, fixing a test: Failure, Cause, Stability evidence
Build/CI/Chore: Affected, Developer and release impact, License and
third-party, Rollback
Revert: Why not fix forward, Original author notified, Follow-up
Release/Dependency: Versions, Upstream changes, Compatibility, Rollback
Other: Why Other
"None" is a valid answer when it is true. Don't restate the diff.
-->
### Testing
<!-- The tests, suites, or commands that ran, what each shows, and where it
ran (locally or in CI). For a fix, the test that fails before it and passes
after. -->
### Notes for Reviewers
<!-- Optional; delete this section if there is nothing to say. Why each
non-obvious edit was made, what you are unsure about, and where to start
reading if Details has no Change map. -->
### Release note
<!-- One paragraph on what users will notice, or None. -->
### Check List (For Author)
<!-- Behavior changed: "No", or "Yes (see Details)" with a Behavior change
field in Details. -->
<!-- Does this need documentation: "No", or "Yes" with the
apache/doris-website PR or the plan. -->
<!-- Pre-submission status: "Ready"; "Partially verified: " and what is not
verified; or "Not ready: " and why, for a Draft PR. -->
- Behavior changed:
- Does this need documentation:
- Pre-submission status:
### AI Disclosure
<!-- Tool: the agent and model, or None. Generated: the parts it wrote
(code, tests, this description), or None. Human review: what you reviewed of
the generated parts, or "Not applicable" without AI tools. You may add "-
Prompts:" to summarize how you directed the agent. -->
- Tool:
- Generated:
- Human review:
--
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]
