Skip to content

fix: preserve input strictness in from_unixtime - #26414

Merged
jayzhan211 merged 2 commits into
apache:mainfrom
haohuaijin:hj/fix-from-unixtime-strict-ordering
Oct 18, 2026
Merged

jayzhan211 merged 2 commits into
apache:mainfrom
haohuaijin:hj/fix-from-unixtime-strict-ordering

Conversation

@haohuaijin

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Found while reviewing #26111; no separate issue. This fix is independent of that PR.

Rationale for this change

from_unixtime(CAST(seconds AS BIGINT)) could incorrectly eliminate a multi-key sort. Ordered input rows (seconds, id) = (1.1, 1), (1.9, 0) produce the same timestamp, but the IDs must be reordered to 0, 1.

What changes are included in this PR?

Propagate the first argument's strict ordering property instead of unconditionally returning true.

What is the testing strategy for this PR?

Three compact SQL cases in datetime/timestamps.slt check the result ordering with and without a timezone, including NULLs, and verify that direct integer inputs still avoid an extra sort. The two regression cases fail without the fix.

Validated with the targeted SQL test file, all 10 from_unixtime unit tests, formatting, and git diff --check.

Are there any user-facing changes?

Yes. Queries with lossy nested inputs to from_unixtime retain required sorts and return correctly ordered rows.

@haohuaijin haohuaijin changed the title fix: preserve input strictness in from_unixtime fix: preserve input strictness in from_unixtime Oct 16, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.88%. Comparing base (1da3cf0) to head (2ffb29a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26414      +/-   ##
==========================================
- Coverage   82.89%   82.88%   -0.01%     
==========================================
  Files        1153     1153              
  Lines      457487   457487              
  Branches   457487   457487              
==========================================
- Hits       379213   379200      -13     
- Misses      55264    55274      +10     
- Partials    23010    23013       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jayzhan211 jayzhan211 left a comment

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.

Thanks @haohuaijin , LGTM

@jayzhan211
jayzhan211 added this pull request to the merge queue Oct 18, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 18, 2026
@haohuaijin

Copy link
Copy Markdown
Contributor Author

Thanks for your reviews @jayzhan211 , i checked the merge queue fail, look like merge queue itself have some issue, need you help to enqueue again.

@jayzhan211
jayzhan211 added this pull request to the merge queue Oct 18, 2026
Merged via the queue into apache:main with commit 5b5edc7 Oct 18, 2026
42 checks passed
@haohuaijin
haohuaijin deleted the hj/fix-from-unixtime-strict-ordering branch October 18, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt) v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants