supabase-community / postgrest-go

Isomorphic Go client for PostgREST. (Now Updating)
https://supabase.io
Apache License 2.0
170 stars 27 forks source link

Reassign transformer methods to FilterBuilder #25

Closed boatilus closed 2 years ago

boatilus commented 2 years ago

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 to FilterBuilder. As you'd expect, this lets you do:

c.From("users").Select("*", "", false).Limit(10)

Of course, it also somewhat unfortunately lets you do:

c.From("users").Select("*", "", false).Limit(10).Eq("name", "sean")

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 (or nil to get the defaults).

type OrderOpts struct {
    Ascending    bool
    NullsFirst   bool
    ForeignTable string
}

func (f *FilterBuilder) Order(column string, opts *OrderOpts) *FilterBuilder

This also roughly mirrors the interface in postgres-js. I'm unsure whether it'd be useful to also update Limit and Range similarly, given they both also currently accept foreignTable.

A full PR would need tests/docs -- this is just aimed at opening up discussion.

yusufpapurcu commented 2 years ago

This solution LGTM. Also requiring struct is a nice solution. But I think we don't need same thing in Limit and Range.

yusufpapurcu commented 2 years ago

LGTM! Thanks for support @boatilus 💯