fix unix_timestamp - #27007
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
…n/matrixone into 0812-fix-unix-timestamp
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Reviewed exact head f489ccf. The typed TIMESTAMP/DATETIME path now returns an exact decimal for fractional temporal inputs while preserving the existing integer result for scale 0 and the no-argument form. I checked planner scale propagation, DATETIME-to-TIMESTAMP timezone conversion, microsecond representation, zero/NULL/pre-epoch handling, constant/type behavior, and the full affected function package. All pkg/sql/plan/function tests pass and CI is green; no blocking correctness, lifecycle, or performance issue found.
Merge Queue Status
This pull request spent 1 hour 31 minutes 42 seconds in the queue, with no time running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks:
HintYou may have to fix your CI before adding the pull request to the queue again. Tick the box to put this pull request back in the merge queue (same as
|
There was a problem hiding this comment.
Reviewed exact head f489ccf. The typed TIMESTAMP/DATETIME path now preserves fractional seconds as DECIMAL(38,6) while scale-0 and no-argument calls retain the existing INT64 behavior. I checked overload/type-scale propagation, DATETIME session-time-zone conversion, NULL/zero/pre-epoch boundaries, Decimal128 range, and normal-call compatibility; no blocking correctness, performance, or lifecycle issue found. CI is green. The focused local function test could not run because this isolated checkout does not contain cgo/libmo.dylib.
aptend
left a comment
aptend
left a comment
There was a problem hiding this comment.
Deep review completed on exact head f489ccfe545685381ea0b5e65717b531a14113ec. The typed TIMESTAMP/casted DATETIME path now derives a decimal result for fractional temporal scales and converts the internal epoch-microsecond representation directly, while preserving the existing integer/no-argument behavior at scale 0. I checked return-type and cast-scale propagation, timezone conversion for DATETIME, decimal scale/representation, zero/NULL/pre-epoch handling, upper-range safety, round-trip behavior, and existing review history. The full pkg/sql/plan/function suite and go vet pass with the required CGO artifacts, CI is green, and I found no blocking correctness or compatibility issue.
5 participants
What type of PR is this?
Which issue(s) this PR fixes:
issue #26979
What this PR does / why we need it:
• 本分支主要修改：
修复 UNIX_TIMESTAMP() 对 typed TIMESTAMP(6) / DATETIME(6) 丢失微秒的问题。
TIMESTAMP/DATETIME 参数 scale > 0 时，UNIX_TIMESTAMP 返回 DECIMAL128(38,6)，保留 fractional seconds。
scale = 0 时仍保持原来的 INT64 秒级返回。
执行路径不再对 fractional temporal 直接调用 Timestamp.Unix()，改为用 epoch microseconds 构造 decimal 结果。
补充 UT 覆盖：
补充 BVT：