Skip to content

Conversation

@gefjon
Copy link
Contributor

@gefjon gefjon commented Oct 31, 2023

Macro generation of filter_by_column on non-unique columns is currently broken; this will stay in draft until that's fixed, as it depends on the same code path.

Description of Changes

  • Refactor bindings::query::delete_by_field to require FilterableValue and return u32 count of deleted rows.
  • Add bindings::query::delete_by_unique_field with the previous behavior of delete_by_field, i.e. requiring UniqueValue and returning bool with true = deleted. This is implemented in terms of delete_by_field.
  • Macro-generated unique filter funcs call delete_by_unique_field rather than delete_by_field.
  • The macro now generates non-unique delete functions alongside the existing non-unique filter functions for non-unique fields with primitive types.

Note that this PR does not include any host-side changes; the diff is localized to the bindings and bindings-macro crates.

API and ABI

  • This is a breaking change to the module ABI
  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI
  • This is a breaking change to the SDK API

If the API is breaking, please state below what will break

This is an additive-only change to the module API, and leaves the module ABI untouched.

Expected complexity level and risk

2 - a reviewer should be prepared to verify that the host-side implementation of delete_by_col_eq correctly handles the existence of multiple matching rows.

How complicated do you think these changes are? Grade on a scale from 1 to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex change.

This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning ways.

… macro

- Refactor `bindings::query::delete_by_field` to require `FilterableValue`
  and return `u32` count of deleted rows.
- Add `bindings::query::delete_by_unique_field`
  with the previous behavior of `delete_by_field`,
  i.e. requiring `UniqueValue` and returning `bool` with true = deleted.
  This is implemented in terms of `delete_by_field`.
- Macro-generated unique filter funcs call `delete_by_unique_field`
  rather than `delete_by_field`.
- The macro now generates non-unique delete functions
  alongside the existing non-unique filter functions
  for non-unique fields with primitive types.
@gefjon
Copy link
Contributor Author

gefjon commented Feb 16, 2024

Superseded by #859

@gefjon gefjon closed this Feb 16, 2024
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