Skip to content

Conversation

@armenzg
Copy link
Member

@armenzg armenzg commented Nov 10, 2025

Changes included:

  • Do not suggest a default value when using options.get()
  • When filtering on columns, check that there's a composite index

Changes included:
* Do not suggest a default value when using `options.get()`
* When filtering on columns, check that there's a composite index
batch_size = options.get("deletions.group-hash-metadata.batch-size")

# WRONG: Redundant default value
batch_size = options.get("deletions.group-hash-metadata.batch-size", 1000)
Copy link
Member Author

Choose a reason for hiding this comment

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

I keep getting suggested to do it this way when coding.

4. Add indexes for queries on 1M+ row tables
5. Use `db_index=True` or `db_index_together`

#### Composite Index Strategy: Match Your Query Patterns
Copy link
Member Author

Choose a reason for hiding this comment

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

This is one of the most repeating issues I have seen in the deletions code yet I introduced the same problem my self! (using id and seer_matched_hash_id in the same query):
#102960 (comment)

@armenzg armenzg marked this pull request as ready for review November 10, 2025 13:39
Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

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

looks reasonable to me.

@armenzg armenzg merged commit 7616b7f into master Nov 10, 2025
37 checks passed
@armenzg armenzg deleted the 11_10/agents_changes/armenzgh branch November 10, 2025 15:57
Jesse-Box pushed a commit that referenced this pull request Nov 12, 2025
Changes included:
* Do not suggest a default value when using `options.get()`
* When filtering on columns, check that there's a composite index
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
Changes included:
* Do not suggest a default value when using `options.get()`
* When filtering on columns, check that there's a composite index
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.

3 participants