Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Type-safe where clauses #426

Closed
aarondl opened this issue Nov 15, 2018 · 15 comments
Closed

Feature: Type-safe where clauses #426

aarondl opened this issue Nov 15, 2018 · 15 comments

Comments

@aarondl
Copy link
Member

aarondl commented Nov 15, 2018

One of the problems in sqlboiler as it stands today is that there's no nice way to do where statements. It would be much nicer if there were type safe where statements for each column created. So for example if I have a model that looks like:

create table users (
   id   serial not null primary key,
   name text not null,
)

Which generates a model:

type User struct {
  ID   int
  Name string
}

It would be nice to be able to create queries that did something like:

users, err := models.Users(models.UserWhere.NameEQ("hello")).All(ctx, db)

Of course for integral types we could also generate the full gamut of < > <= >= != with the suffixes GT, LT, GTE, LTE, NEQ or something similar.

This allows us to do the exact right thing with whatever type it is, for example when things are null the query can be difficult to create because SQL is a terrible language and decided that = null should be written as is null for some reason so you wind up creating boilerplate to deal with this as alluded to here: #417

This is not a breaking change and can be implemented at any time.

@aarondl
Copy link
Member Author

aarondl commented Jan 2, 2019

Some work has been done on this now.

For those watching this issue as well as those who have opened related issues and prolific contributors:
@ceshihao @glerchundi @shousper @pennersr I invite you to comment on this work. At the bottom
of this issue I'll have some questions that you might answer directly and of course any adhoc feedback
is especially welcome.

Currently you only get the EQ/NE for all types. For Go primitive types (int, rune, byte etc) you additionally get the LT/LTE/GT/GTE.
My concerns with it are mainly the amount of methods being generated for a given database.

Currently for a table that has an ID and a Name its generating helpers like this:

Done stuff

var UserWhere = struct {
        IDEQ    func(int) qm.QueryMod
        IDNEQ   func(int) qm.QueryMod
        IDLT    func(int) qm.QueryMod
        IDLTE   func(int) qm.QueryMod
        IDGT    func(int) qm.QueryMod
        IDGTE   func(int) qm.QueryMod
        NameEQ  func(string) qm.QueryMod
        NameNEQ func(string) qm.QueryMod
        NameLT  func(string) qm.QueryMod
        NameLTE func(string) qm.QueryMod
        NameGT  func(string) qm.QueryMod
        NameGTE func(string) qm.QueryMod
}{
        IDEQ:    func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.EQ, x) },
        IDNEQ:   func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.NEQ, x) },
        IDLT:    func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.LT, x) },
        IDLTE:   func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.LTE, x) },
        IDGT:    func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.GT, x) },
        IDGTE:   func(x int) qm.QueryMod { return qmhelper.Where(`id`, qmhelper.GTE, x) },
        NameEQ:  func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.EQ, x) },
        NameNEQ: func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.NEQ, x) },
        NameLT:  func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.LT, x) },
        NameLTE: func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.LTE, x) },
        NameGT:  func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.GT, x) },
        NameGTE: func(x string) qm.QueryMod { return qmhelper.Where(`name`, qmhelper.GTE, x) },
}

Which allows queries like this:

u, err := models.Users(
  models.UserWhere.NameEQ("Hello"),
).All(context.Background(), db)

Or even this:

where := &models.UserWhere
u, err := models.Users(where.NameEQ("Hello")).All(context.Background(), db)

This of course fixes the is null vs = problem as you can simply do:

u, err := models.Users(
  models.UserWhere.NameEQ(myNullString),
).All(context.Background(), db)

The downside of this of course is checking for null (or not null) itself is a bit onerous:

  models.UserWhere.NameEQ(null.NewString("", false)),
  models.UserWhere.NameNEQ(null.NewString("", false)),

Perhaps we need to add an IsNull/IsNotNull, but that's more functions:

u, err := models.Users(
  models.UserWhere.NameIsNull(),
).All(context.Background(), db)

Additional work

I want to do two more things with where clauses in sqlboiler.

  1. Clean up how Or and And work with where

    // Currently
    u, err := models.Users(qm.Where("name = 'john'"), qm.Or("name = 'fred'"))
    
    // New API:
    u, err := models.Users(qm.Where("name = 'john'"), qm.Or2(qm.Where("name = 'fred'"))
    
    // Final API in v4 (when we can do breaking changes):
    u, err := models.Users(qm.Where("name = 'john'"), qm.OrWhere("name = 'fred'"))
    u, err := models.Users(qm.Where("name = 'john'"), qm.Or(qm.Where("name = 'fred'"))

    This is mainly for consistency so you can use the new helpers as well as the expression nesting below

    Using the new Or with a type-safe helper

    // With Or2 you can do this:
    where := &models.UserWhere
    u, err := models.Users(where.NameEQ("john"), qm.Or2(where.NameEQ("fred"))
  2. Add better support for nesting where clauses

    Grouping statements with Expr

    u, err := models.Users(qm.Expr(qm.Where("name = 'john'"), qm.Where("age > 5")), qm.Or(qm.Where("name = 'fred'"))
    // Produces:
    // select * from users where (name = 'john' and age > 5) or name = 'fred'

Questions

  1. Do we need to have as IsNull and IsNotNull helper for nullable types?
  2. Are we okay with breaking Or into Or and Or2 for v3, then Or and OrWhere in v4?
  3. The name of Expr is a bit.. odd but I don't know what else to call it. Suggestions welcome.
  4. Any suggestions to be able to clean this up a bit so we're not generating so many functions?
  5. Is there maybe a better way to namespace these helper functions in Go that I'm not seeing (other than separate package, though we could consider that too if someone had a nice enough way to do it)?

@shousper
Copy link

shousper commented Jan 3, 2019

Love it!

I think you could generate type-specific "where helpers" that should cut down the per-table code size, for example:

type IntWhere struct {
	field string
}

func (w IntWhere) Eq(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.EQ, x) }
func (w IntWhere) Neq(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.NEQ, x) }
func (w IntWhere) Lt(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.LT, x) }
func (w IntWhere) Lte(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.LTE, x) }
func (w IntWhere) Gt(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.GT, x) }
func (w IntWhere) Gte(x int) qm.QueryMod { return qmhelper.Where(w.field, qmhelper.GTE, x) }

type StringWhere struct {
	Eq  func(string) qm.QueryMod
	Neq func(string) qm.QueryMod
	Lt  func(string) qm.QueryMod
	Lte func(string) qm.QueryMod
	Gt  func(string) qm.QueryMod
	Gte func(string) qm.QueryMod
}
func StringWhereFor(field string) StringWhere {
	return StringWhere{
		Eq:  func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.EQ, x) },
		Neq: func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.NEQ, x) },
		Lt:  func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.LT, x) },
		Lte: func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.LTE, x) },
		Gt:  func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.GT, x) },
		Gte: func(x string) qm.QueryMod { return qmhelper.Where(field, qmhelper.GTE, x) },
	}
}

var UserWhere = struct {
	ID IntWhere
	Name StringWhere
}{
	ID: IntWhere{`id`},
	Name: StringWhereFor(`name`),
}

I'm not sure on the reason behind the anonymous struct vs fixed struct, but I added both purely to demonstrate. Usage for this wouldn't change much, just an extra dot.

u, err := models.Users(
	models.UserWhere.Name.Eq("Hello"),
).All(context.Background(), db)

I used TitleCase to match go's convention for public function names. If you use function reference fields in all caps I'm not sure how some linters will feel about it.

Also not using pointers for the "where helpers" so we don't need to worry about nils.

It all looks great to me though! Awesome stuff :)

@aarondl
Copy link
Member Author

aarondl commented Jan 3, 2019

I think that this says it should be EQ/NEQ/LT/LTE etc:
https://github.com/golang/go/wiki/CodeReviewComments#initialisms
I could be convinced otherwise.

@shousper
Copy link

shousper commented Jan 3, 2019

Good point! I'd forgotten that one. It's an odd case though, I don't know if "EQ" or "NEQ" are initialisms, are they? The others definitely are.

No matter, I'm an ex-Java guy so I still gravitate to fully qualified names, lol.

Do you think type-specific helpers might work well?

@aarondl
Copy link
Member Author

aarondl commented Jan 3, 2019

Yes definitely. I had thought about them earlier but for whatever reason gave up on them (I think I didn't want to do a conditional for them in the templates because it's an annoying amount of template code.) They're also not always necessary so I had thought about conditional generation based on what types were used but I'll probably just throw them in the new qmhelper package I made for this work and call it good.

aarondl added a commit that referenced this issue Jan 6, 2019
@aarondl
Copy link
Member Author

aarondl commented Jan 6, 2019

There's a new where branch with an implementation. I think this is the one we'll keep.

@shousper Ended up doing the work to be able to generate each where helper one time and re-use it from everywhere that needs that type of where helper. This cuts down on the amount of generated code a ton I'd wager.

Have yet to do the qm.Expr, qm.Or2 and qm.And2 implementations.

@aarondl
Copy link
Member Author

aarondl commented Jan 9, 2019

All implementations complete on the dev branch. Please take a look. If there are problems with the current implementation let me know before release. Release will be in a few days (this is arbitrary, there's just enough changes that I want to publish them to master now, many fixes have been done).

@aarondl aarondl closed this as completed Jan 9, 2019
@autarch
Copy link
Contributor

autarch commented Feb 7, 2019

This is a great new feature, but I note that it's not reflect in the README.md file at all.

@aarondl
Copy link
Member Author

aarondl commented Feb 7, 2019

Hi @autarch. Many sections have had their examples duplicated with the alternative syntax:

https://github.com/volatiletech/sqlboiler/#query-building
https://github.com/volatiletech/sqlboiler/#query-mod-system
https://github.com/volatiletech/sqlboiler/#constants

And it's original documentation is in the Constants section. Admittedly I wasn't sure where to put it when I finished the feature, so I just put it where it seemed relevant.

I also notice your fork is in the ActiveState org, I also worked there for a time, small world :)

@autarch
Copy link
Contributor

autarch commented Feb 7, 2019

Ah, I see. It's easy to miss that if you've read the docs before that was added. I think this may just be an issue with docs in a README.md as opposed to a more structured set of doc pages (which Go doesn't help encourage, unfortunately).

And yes, apparently it's a very small world! ActiveState isn't a big company, so it's interesting to meet others who've been here.

@e-nikolov
Copy link

e-nikolov commented Feb 21, 2019

I see qm.Or2() that accepts a query mod, but no qm.And2(). Is that intended?

@aarondl
Copy link
Member Author

aarondl commented Feb 21, 2019

@e-nikolov And2 didn't seem required as the default operand is and. Expr will also do And. Simply listing them next to each other is enough.

@e-nikolov
Copy link

I see, thanks, maybe would be useful to mention in the examples (unless it is already and I missed it).

@9214
Copy link

9214 commented Apr 20, 2022

Sorry for necroposting, but Query mod system example in README has an error related to requested And2:

Where("(name=? and age=?) or (age=?)", "John", 5, 6)
//              ↑ 'and' here                      ↑ '6' here

But then:

Where(
  Expr(
    models.PilotWhere.Name.EQ("John"),
    Or2(models.PilotWhere.Age.EQ(5)), // ← no 'And2' here
  ),
  Or2(models.PilotAge), // ← no 'models.PilotWhere.Age.EQ(6)' here
)

@stephenafamo
Copy link
Collaborator

Oh! The README is wrong 🤦🏾 . It should be:

Where(
  Expr(
    models.PilotWhere.Name.EQ("John"),
    models.PilotWhere.Age.EQ(5)), // ← it uses "AND" by default.
  ),
  Or2(models.PilotWhere.Age.EQ(6)), // ← no 'models.PilotWhere.Age.EQ(6)' here
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants