Skip to content

fix: avoid mutating DatetimeRange condition in check_datetime_range#1163

Closed
jnMetaCode wants to merge 1 commit into
qdrant:masterfrom
jnMetaCode:fix/datetime-range-mutation
Closed

fix: avoid mutating DatetimeRange condition in check_datetime_range#1163
jnMetaCode wants to merge 1 commit into
qdrant:masterfrom
jnMetaCode:fix/datetime-range-mutation

Conversation

@jnMetaCode

Copy link
Copy Markdown

Summary

  • Fix check_datetime_range in qdrant_client/local/payload_filters.py to avoid mutating its condition argument in-place.
  • Previously, the function reassigned condition.lt, condition.lte, condition.gt, and condition.gte with timezone-aware conversions, permanently altering the caller's DatetimeRange filter object.
  • Now uses local variables for the timezone-aware boundaries, leaving the original condition unchanged.

Problem

When check_datetime_range is called with a DatetimeRange condition containing timezone-naive datetime or date boundaries, it converts them to timezone-aware datetime objects directly on the condition object (lines 138-141). This has two side effects:

  1. Type mutation: date objects on the condition are silently converted to datetime objects, which could break downstream code that inspects the filter type.
  2. Persistent modification: The caller's filter object is permanently altered. While this is idempotent for subsequent calls (already-converted values are unchanged), it violates the expectation that a predicate function (check_*) should not modify its inputs.

Fix

Use local variables (lt, lte, gt, gte) instead of reassigning attributes on the condition object. The comparison logic remains identical.

Test plan

  • Existing tests for datetime range filtering should continue to pass.
  • Verify that reusing a DatetimeRange filter across multiple check_datetime_range calls no longer mutates the filter.

`check_datetime_range` was modifying its `condition` parameter in-place
by reassigning `condition.lt`, `condition.lte`, `condition.gt`, and
`condition.gte` with timezone-aware versions. This caused unintended
side effects: callers reusing the same `DatetimeRange` filter would
find their `date` boundaries silently converted to `datetime` objects,
and the filter object permanently altered.

Use local variables instead so the original condition remains unchanged.

Signed-off-by: JiangNan <[email protected]>
@netlify

netlify Bot commented Mar 8, 2026

Copy link
Copy Markdown

Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit 37cbad7
🔍 Latest deploy log https://app.netlify.com/projects/poetic-froyo-8baba7/deploys/69ad4b7dae675a0008cde3ea
😎 Deploy Preview https://deploy-preview-1163--poetic-froyo-8baba7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Mar 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c37639c-71b6-4c20-8a6e-14c1c24c9e73

📥 Commits

Reviewing files that changed from the base of the PR and between e7101dc and 37cbad7.

📒 Files selected for processing (1)
  • qdrant_client/local/payload_filters.py

📝 Walkthrough

Walkthrough

This change refactors the check_datetime_range function in qdrant_client/local/payload_filters.py to avoid mutating the input condition object. Instead of directly assigning values to the condition's lt, lte, gt, and gte attributes, the function now uses local temporary variables for these values and performs the same comparison logic using these temporaries. The behavior and output remain identical, but the input condition object is no longer modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: avoiding mutation of DatetimeRange condition in check_datetime_range function.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the problem, solution, and testing approach for the mutation fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@joein

joein commented Mar 13, 2026

Copy link
Copy Markdown
Member

Hey @jnMetaCode

Thanks for pointing it out and providing a fix!

I'll recreate the PR to point to dev branch as specified the PR template

@joein

joein commented Mar 13, 2026

Copy link
Copy Markdown
Member

closing in favour of #1169

@joein joein closed this Mar 13, 2026
@jnMetaCode

Copy link
Copy Markdown
Author

Thanks @joein for picking this up and recreating the PR against dev! Glad the fix was useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants