Reassign transformer methods to FilterBuilder #25
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In lieu of a more ergonomic option within Go's type system (that feels reasonable) to resolve #8, I'm proposing removing
TransformBuilder
and simply reassigning its associated methods toFilterBuilder
. As you'd expect, this lets you do:Of course, it also somewhat unfortunately lets you do:
Which obviously looks weird.
Since the transformer methods aren't usable in the current release, this is probably also a good time to discuss some of ergonomics around the transformer methods. Having four params with type similarities in
Order
, for example, feels a bit unnatural, so I'd propose having it instead accept a struct of options (ornil
to get the defaults).This also roughly mirrors the interface in postgres-js. I'm unsure whether it'd be useful to also update
Limit
andRange
similarly, given they both also currently acceptforeignTable
.A full PR would need tests/docs -- this is just aimed at opening up discussion.