New "editor" helper to avoid column lists for simple inserts/updates

[falconandy]

Motivation

Allow to avoid boring column list in inserts/updates (especially if we want to process empty strings, false values, zeros).

The initial commit doesn't add documentation/tests and doesn't contain InsertG/UpdateG, InsertP/UpdateP implementation. I'll add it later if the new concept will be approved.

Current code

apiKey, err := models.FindAPIKey(ctx, s.db, int(r.KeyId))
if err != nil {
	...
}
apiKey.Disabled = r.Disabled
apiKey.Title = r.Title
_, err = apiKey.Update(ctx, s.db, boil.Whitelist(models.APIKeyColumns.Disabled, models.APIKeyColumns.Title))

Suggested code

apiKey, err := models.FindAPIKey(ctx, s.db, int(r.KeyId))
if err != nil {
	...
}
_, err = apiKey.E().Disabled(r.Disabled).Title(r.Title).Update(ctx, s.db)

Suggested code (multi-line)

apiKey, err := models.FindAPIKey(ctx, s.db, int(r.KeyId))
if err != nil {
	...
}
_, err = apiKey.E().
	Disabled(r.Disabled).
	Title(r.Title).
	Update(ctx, s.db)

Generated code

func (o *APIKey) E() *APIKeyE {
	return &APIKeyE{S: o}
}

func APIKeyEditor() *APIKeyE {
	return &APIKeyE{S: &APIKey{}}
}

type APIKeyE struct {
	S       *APIKey
	columns []string
}

func (e *APIKeyE) Insert(ctx context.Context, exec boil.ContextExecutor) error {
	return e.S.Insert(ctx, exec, boil.Whitelist(e.columns...))
}

func (e *APIKeyE) Update(ctx context.Context, exec boil.ContextExecutor) (int64, error) {
	return e.S.Update(ctx, exec, boil.Whitelist(e.columns...))
}

func (e *APIKeyE) Disabled(disabled bool) *APIKeyE {
	e.S.Disabled = disabled
	e.columns = append(e.columns, APIKeyColumns.Disabled)
	return e
}

func (e *APIKeyE) Title(title string) *APIKeyE {
	e.S.Title = title
	e.columns = append(e.columns, APIKeyColumns.Title)
	return e
}

[aarondl]

This is actually an interesting idea as my own project is covered with these sorts of repetitive lists. My main concern is that it will add a lot of additional generated code and yet another magic struct field (having to have L & R are bad enough).

I think if we wanted to accept this proposal to avoid the additional magic struct field we'd do:

models.EditUser(u).Username("hello").Update(...)
// instead of:
u.E().Username("hello").Update(...)

What do you think?

[falconandy]

1. The name EditUser can conflict with an actual entity name. Currently I use models.UserEditor() to create the wrapper for a new user (to avoid long expression like (&models.User{}).E()) - the name UserEditor can conflict too.
2. To create the editor for a new user we could write models.EditUser(nil). Or models.EditNewUser()?
3. Currently in some places of my code I create the editor for a new/updated entity and set some fileds on service/router level, then I send the editor (type models.UserE) to "domain" logic that could update other fields and invokes Update()/Insert(). Sometimes I need access to entity field values from the editor object - in this case I use the S field (means "Source", short name to avoid conflicts with setter names)
4. Maybe setter names should have prefix Set: models.EditUser(u).SetUsername("hello").Update(...) - in this case the S field could be renamed to Source or to actual entity name (User). Additionally it could help to avoid runtime errors - during the refactoring code like models.Users(qm.Where("kind=?", user.Kind)) fails in runtime if the user variable becomes a editor. Need to change to models.Users(qm.Where("kind=?", user.S.Kind))

[aarondl]

1. We can use whatever name we'd like (alias support in sqlboiler is good nowadays).
2. The nil seems fine rather than creating more names to alias around.
   3 & 4. Editing or Source is fine, no S. Set -should- be fine, but it does conflate a bit with the relationship helpers (SetVideos) for example.

So I'm remiss @falconandy and must apologize. For delaying this for such a long time. I honestly haven't been able to respond because this feature has me very torn.

In terms of pros and cons it looks like this to me:

Pros:

• Fewer bugs due to de-sync'd whitelists & struct assignments (you can easily forget to add a whitelist column entry)
• Less duplication, means using sqlboiler becomes slightly easier

Cons:

• It's another way to do something we already have a way to do it
• A bit magical because you don't really know what's going on under the hood from looking at it
• It does run against our "Go-like feel" goal in that everything is just structs, more elaboration below.
• A fairly significant chunk of additional generated code (I would like to see a percentage difference on this, how many lines does this add to the test schemas we have?)

When we first created sqlboiler one thing we really wanted was that Go like feel. We just wanted to use plain structs because that's a very natural way to code in Go. We wanted normal-ish types (null package sucks but necessary evil in this language) and I think it does make it fairly nice to work with. This does bend that a little bit.

Having said that the idea isn't really bad, and there is some safety for those who would choose it.

I'd invite some of our contributors to have a say on the feature because I'm having a very difficult time determining if it should make it in myself. @nadilas, @glerchundi, @ceshihao, @gemscng if any of you have comments or want to slap the +1 -1 button on this comment please do so.

[ceshihao]

I am ok if it is not breaking one.

My only concern is more and more generated codes. It will make a slow build or large binary, even if most of codes are actually not used in my app.

Maybe some hugh changes could be implemented in a tailorable way.

[aarondl]

That's a good point. It could be a flag option as we have for G/P/Context. It's default state would be up for debate of course.

[gemscng]

1. We can use whatever name we'd like (alias support in sqlboiler is good nowadays).
2. The nil seems fine rather than creating more names to alias around.
   3 & 4. Editing or Source is fine, no S. Set -should- be fine, but it does conflate a bit with the relationship helpers (SetVideos) for example.

So I'm remiss @falconandy and must apologize. For delaying this for such a long time. I honestly haven't been able to respond because this feature has me very torn.

In terms of pros and cons it looks like this to me:

Pros:

• Fewer bugs due to de-sync'd whitelists & struct assignments (you can easily forget to add a whitelist column entry)
• Less duplication, means using sqlboiler becomes slightly easier

Cons:

• It's another way to do something we already have a way to do it
• A bit magical because you don't really know what's going on under the hood from looking at it
• It does run against our "Go-like feel" goal in that everything is just structs, more elaboration below.
• A fairly significant chunk of additional generated code (I would like to see a percentage difference on this, how many lines does this add to the test schemas we have?)

When we first created sqlboiler one thing we really wanted was that Go like feel. We just wanted to use plain structs because that's a very natural way to code in Go. We wanted normal-ish types (null package sucks but necessary evil in this language) and I think it does make it fairly nice to work with. This does bend that a little bit.

Having said that the idea isn't really bad, and there is some safety for those who would choose it.

I'd invite some of our contributors to have a say on the feature because I'm having a very difficult time determining if it should make it in myself. @nadilas, @glerchundi, @ceshihao, @gemscng if any of you have comments or want to slap the +1 -1 button on this comment please do so.

Thanks @aarondl invite me. Whitelist only need if we use has a non-nullable boolean field default to true. In most cases, we only need to use infer. Introducing unnecessary newpattern will make the library harder and harder to maintain. Current go-like way is fine.

[nadilas]

Thanks @aarondl for letting us weigh in on this. I'm with @gemscng on this one, it would introduce a "new" handling to an already established process in a "black magic" way as you described. I feel the cons outweigh the benefit right now.

[falconandy]

Thank you all for feedback. I'll do it in a separate repository

[aarondl]

@falconandy I appreciate the attempt. It was quite a good idea. I think you'll benefit a lot from using it in your own projects. Consider publishing them as an alternative to our default templates :)