Skip to content

Users/bambriz/feedrangehotfix0 #41270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 23, 2025

Conversation

bambriz
Copy link
Member

@bambriz bambriz commented May 22, 2025

Description

This PR fixes an issue where querychangefeed doesn't return items that have PartitionKey Values that use Hash V1. This also fixes issue where one couldn't query changefeed for specific HPK PK values that weren't prefix pks. A Murmurhash3 32bit x86 algorithm was implemented and made to match what backend expects for the EPK of Hash V1 Partition Key Values.

FabianMeiswinkel and others added 2 commits May 20, 2025 15:55
This fixes an issue where querychangefeed doesn't return items that have PartitionKey Values that use Hash V1. This also fixes issue where one couldn't query changefeed for specific HPK PK values that weren't prefix pks.
@Copilot Copilot AI review requested due to automatic review settings May 22, 2025 22:11
@bambriz bambriz requested a review from a team as a code owner May 22, 2025 22:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for legacy Hash V1 partition key hashing (using MurmurHash3 32-bit) so change feed queries return items for containers using V1 keys, and it updates both sync and async code paths and tests accordingly.

  • Implemented V1 hashing logic in PartitionKey (_truncate_for_v1_hashing, murmurhash3_32, and binary encoding)
  • Propagated partition key version in container/client constructors and query logic
  • Added/updated sync and async tests to validate change feed behavior on Hash V1, Hash V2, and hierarchical keys
  • Updated CHANGELOG with new release entry

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sdk/cosmos/azure-cosmos/tests/test_changefeed_partition_key_variation_async.py New async tests for change feed across Hash V1/V2 and HPK cases
sdk/cosmos/azure-cosmos/tests/test_changefeed_partition_key_variation.py New sync tests for change feed across Hash V1/V2 and HPK cases
sdk/cosmos/azure-cosmos/azure/cosmos/partition_key.py Added V1 hashing, truncate, 32-bit Murmur, and binary encoding
sdk/cosmos/azure-cosmos/azure/cosmos/container.py Forwarded version when constructing PartitionKey
sdk/cosmos/azure-cosmos/azure/cosmos/aio/_cosmos_client_connection_async.py Forwarded version in async client partition key logic
sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py Forwarded version in async container partition key logic
sdk/cosmos/azure-cosmos/azure/cosmos/_cosmos_murmurhash3.py Added murmurhash3_32 implementation
sdk/cosmos/azure-cosmos/azure/cosmos/_cosmos_integers.py Introduced _UInt32 and encode/decode helpers for 32-bit values
sdk/cosmos/azure-cosmos/azure/cosmos/_cosmos_client_connection.py Updated sync client to check and use version in prefix queries
sdk/cosmos/azure-cosmos/CHANGELOG.md Documented the V1 change feed fix in 4.12.0b2 release notes
Comments suppressed due to low confidence (1)

sdk/cosmos/azure-cosmos/azure/cosmos/partition_key.py:211

  • The with (BytesIO() as ms): syntax is invalid. It should be with BytesIO() as ms: to correctly open the context manager.
with (BytesIO() as ms):

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FabianMeiswinkel FabianMeiswinkel merged commit 7a4ee24 into Azure:main May 23, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants