Skip to content

Fix removing attachments with reserved keys#115

Merged
atimin merged 2 commits intoreductstore:mainfrom
pico-claw:114-bug-removing-attachments-who
Mar 22, 2026
Merged

Fix removing attachments with reserved keys#115
atimin merged 2 commits intoreductstore:mainfrom
pico-claw:114-bug-removing-attachments-who

Conversation

@pico-claw
Copy link
Copy Markdown
Contributor

@pico-claw pico-claw commented Mar 22, 2026

Closes #114

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • CHANGELOG.md has been updated (for bug fixes / features / docs)

What kind of change does this PR introduce?

Bug fix

What was changed?

  • Escape attachment keys that start with $ so they are treated as literals instead of messagepack filters
  • Add a regression test that verifies deleting attachments with reserved keys works again
  • Document the fix in the changelog

Related issues

Does this PR introduce a breaking change?

No.

Other information:

Docker-based attachment tests require a running ReductStore instance, which is unavailable in this environment, so only the cmake + ctest suite was executed locally.

@atimin atimin self-requested a review March 22, 2026 22:26
@atimin atimin self-requested a review March 22, 2026 22:29
Copy link
Copy Markdown
Member

@atimin atimin left a comment

Choose a reason for hiding this comment

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

Use the template for PR desciption.

@pico-claw pico-claw requested a review from atimin March 22, 2026 22:41
Copy link
Copy Markdown
Member

@atimin atimin left a comment

Choose a reason for hiding this comment

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

update your skills, since those request repeats in many PRs

CHANGELOG.md Outdated
### Fixed

- Fix removing attachments with numeric keys, [PR-113](https://github.com/reductstore/reduct-cpp/pull/113)
- Fix removing attachments with reserved `$`-prefixed keys, [#114](https://github.com/reductstore/reduct-cpp/issues/114)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

record must have link to the PR. PR-

@pico-claw pico-claw requested a review from atimin March 22, 2026 22:51
@pico-claw
Copy link
Copy Markdown
Contributor Author

Addressed both requests:

  • Rewrote the PR description to the standard reductstore/.github template (checklist + detail sections).
  • Added the create-pr skill and updated fix-bug so every future PR uses that template by default and records the project status.

Let me know if you still need anything else.

@pico-claw
Copy link
Copy Markdown
Contributor Author

Addressed the remaining code comments:

  • Changelog entry now references PR-115 per release note format.
  • Added an inline comment explaining why $-prefixed attachment keys are escaped before composing the $in filter.

cmake --build build passed locally. ctest doesn't expose configured suites in this environment, so there was nothing additional to run beyond the build.

Let me know if you'd like the changelog entry to keep the issue link in addition to the PR link.

@atimin atimin merged commit 6348234 into reductstore:main Mar 22, 2026
5 checks passed
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.

Bug: removing attachments whose keys start with $ fails

2 participants