RavenDB-26793 Time-series bulk-insert UTC fix + custom-entity-name control-char guard#303
Merged
Merged
Conversation
…atetimes as UTC Utils.get_unix_time_in_ms used datetime.timestamp(), which interprets a naive datetime in the machine's local timezone, so bulk-insert time-series timestamps were shifted by the local UTC offset on non-UTC machines (e.g. 2h on CET). The regular session path (Utils.datetime_to_string) treats naive datetimes as UTC; do the same here so both paths agree and results are timezone-independent.
RavenDB now rejects control characters in document ids (including the generated HiLo id). The test injected control chars 1..13 into the collection name via a guard that only skipped 14..31, so it failed against current 6.2/7.2 servers. Skip the whole C0 control range (ord <= 31).
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Two client-side fixes surfaced while validating the 6.2.18 / 7.2.5 nightlies against the Python client.
1. Bulk-insert time-series timestamps shifted by local UTC offset (bug)
Utils.get_unix_time_in_ms()useddatetime.timestamp(), which interprets a naive datetime in the machine's local timezone. Bulk-insert time-series timestamps were therefore shifted by the local UTC offset on non-UTC machines (e.g. +2h on CET), whereas the regular session path (Utils.datetime_to_string) treats naive datetimes as UTC. That made ~18test_time_series_bulk_insert/test_typed_bulk_insertcases fail locally (they pass on CI because CI runs in UTC).Fix: treat naive datetimes as UTC in
get_unix_time_in_ms(its only caller is the bulk-insert time-series path), consistent withdatetime_to_string. tz-aware datetimes are unaffected.2.
test_custom_entity_nameguard let control chars through (test)Current 6.2.x / 7.2.x servers now reject control characters in document ids (including the generated
Raven/Hilo/...id) viaDocumentIdWorker.ThrowIdentifierWithControlCharacters. Verified locally: bundled 7.2.1 accepts them (test passes), while both 6.2.18-nightly and 7.2.5-nightly reject them (HTTP 500). The test injected control chars 1..13 into the collection name through a guard that only skipped 14..31; widened it to skip the whole C0 control range (ord(c) <= 31).Verification
black --checkclean.test_custom_entity_name) pass against a local 7.2.5-nightly server.Notes for reviewers
test_custom_entity_nameis a jvm-migrated test; the14 <= c <= 31guard is a straight port, so the JVM / C# / Node suites likely carry the same guard and will start failing against these nightlies — worth fixing across clients or documenting the id contract.4xx. That's a separate server-side consideration, not addressed here.