[pull] main from MaterializeInc:main#1038
Merged
Merged
Conversation
…y marker (#36732) The csv crate strips quoting during parsing, so `decode_copy_format_csv` could not distinguish a quoted NULL marker from an unquoted one (silent data corruption: e.g. with default params, a literal `""` decoded to SQL NULL instead of the empty string), and the end-of-copy marker check treated a quoted `"\."` value as the bare `\.` terminator (silent data loss: the row and every subsequent row were dropped). Switch the decoder to csv-core so we can inspect each field's leading input byte and recover per-field quote state; the NULL-marker and end-of-copy checks both now require the field to be unquoted. The pgwire COPY row scanner had the same quote-blind end-marker check; switch it to match against the raw input bytes for the in-progress record. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Inspired by #36767
### Motivation Fixes CLU-93: applying a fixed timezone offset to a near-maximum timestamp could cause a panic instead of properly returning a "timestamp out of range" error. ### Description The `timezone_timestamp` function was performing unchecked subtraction when applying a timezone offset to a timestamp. When the offset subtraction would overflow the valid timestamp range, this could panic or produce incorrect results. The fix introduces a new `checked_sub_with_leapsecond` helper function (mirroring the existing `checked_add_with_leapsecond`) that safely performs the subtraction with proper overflow checking. This function: - Temporarily removes the nanosecond component to avoid precision loss - Uses `checked_sub_signed` to safely perform the subtraction - Returns `None` if the operation would overflow - Recovers the nanosecond component in the result The caller now properly converts the `Option` result to an `EvalError::TimestampOutOfRange` error instead of panicking. ### Verification Added a regression test in `timezone.slt` that verifies applying a `+15:00` offset to a near-maximum timestamp returns a "timestamp out of range" error rather than panicking. Existing timezone tests continue to pass. https://claude.ai/code/session_019Bu679om3fCETwm3nMLmtN --------- Co-authored-by: Claude <noreply@anthropic.com>
### Motivation
Fixes database-issues#CLU-92: A leap-second timestamptz combined with a
fixed offset that shifts the seconds off `:59` previously panicked
during row encoding.
### Description
The issue occurs because chrono represents leap seconds as nanosecond
values >= 1,000,000,000, but only when the seconds-of-minute is 59. When
a timezone offset shifts a leap-second timestamp to a different second
(e.g., from `:59` to `:00` of the next minute), the resulting
`NaiveTime` becomes unconstructable via
`from_num_seconds_from_midnight_opt` and panics during row encoding.
The fix modifies `checked_add_with_leapsecond` to detect this case:
after applying the offset, if the nanosecond value still represents a
leap second (>= 1,000,000,000) but the resulting time is no longer at
second 59, we fold the leap-second nanoseconds into the next regular
second by adding them as a duration. This ensures the resulting
`NaiveTime` is always valid and representable.
### Verification
Added regression tests in `timestamptz.slt` that verify:
- `timezone('+00:00:01', TIMESTAMPTZ '2024-06-30 23:59:60+00')`
correctly produces `2024-07-01 00:00:01`
- `timezone('-00:00:01', TIMESTAMPTZ '2024-06-30 23:59:60+00')`
correctly produces `2024-06-30 23:59:59`
Both cases previously panicked; they now produce correct results.
https://claude.ai/code/session_01GxFbwLr4Gku9kJ58EscJs5
---------
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )