Sitelet https://web.archive.org/web/20201209034253/https://github.com/volatiletech/sqlboiler/pull/745
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

Add a way to reset queries so they can be reused #745

Open
wants to merge 1 commit into
base: dev
from

Conversation

@stephenafamo
Copy link
Contributor

@stephenafamo stephenafamo commented May 15, 2020

Also add a flag that generates models that resets the query after each finisher.

Fixes #647

When the models are generated with the flag --add-reset, Queries started with models.Pilot() can be reused. For example:

query := models.Users()

users, err := query.Count(req.Context(), s.DB)
fmt.Println(users, err)

count, err := query.Count(req.Context(), s.DB)
fmt.Println(count, err)

Previously that will throw an error.

It also adds the type QueryReset which is the primary way or resetting queries. For example:

query := models.NewQuery(
    qm.From("users"),
)
resetter := q.AddReset()

resetter.Save() // saves what the query was at this point
_ = q.Bind(ctx, db, &obj) // selects all
resetter.Reset() // resets the query to the saved point

qm.Apply(query, qmhelper.Where("active", qmhelper.EQ, true)
resetter.Save() // New save point

_ = q.Bind(ctx, db, &obj) // selects all active users
resetter.Reset() // resets the query to the saved point

queries.SetCount(query)
 = q.Bind(ctx, db, &obj) // counts

This method is also completely backward compatible.

Also add a flag that generates models that resets the query after each finisher.
@stephenafamo
Copy link
Contributor Author

@stephenafamo stephenafamo commented May 27, 2020

Can I get some feedback on this PR?

Copy link
Member

@aarondl aarondl left a comment

Hello @stephenafamo, and thanks for the PR! Apologies for letting this sit for so long.

This PR doesn't work for the reason I highlighted, but I'm also not sure if I'm willing to accept such a syntactically noisy addition. It does solve the problem, but it requires so much additional "stuff" in order to reset a query that it doesn't seem worth it.

Ideally we'd be able to have queries just magically resettable (if there is such a way) and I'd like to prove that that's not possible before pursuing a route like this.


// Save removes the effect of a finisher
func (q *QueryReset) Save() {
q.saved = *q.q

This comment has been minimized.

@aarondl

aarondl Jun 17, 2020
Member

This is insufficient because it does not do a deep copy. If slices/maps are modified in the query it could affect a saved query.

@stephenafamo
Copy link
Contributor Author

@stephenafamo stephenafamo commented Jun 17, 2020

Thanks for the feedback. I could make some changes to the PR, but I think it's better if I get a better sense of how you'd prefer the reset to work.

I think this feature is useful, but I'd like to understand what you have in mind so I see if it is something I can work on.

@aarondl
Copy link
Member

@aarondl aarondl commented Jun 18, 2020 •

My issue with the approach lies in the fact that users will not try that first. They'll first try what andradei tried:

myQuery := models.Things(qm.Where("name=?", someName))

// Use myQuery once, it works.
exists, err := myQuery.Exists(
    context.Background(), db)
    if err != nil {
        return fmt.Errorf("error looking for thing: %w", err)
    }

// Use myQuery again, doesn't work.
thing, err := myQuery.One(context.Background(), db)
if err != nil {
    return fmt.Errorf("error getting existing Thing: %w", err)
}

And only after they encounter errors/problems will they seek a resolution and finally find the resetter and learn how that works and use it.

Perhaps that's better than "they encounter errors/problems and seek a solution only to find one doesn't exist" but does there exist a design where we could simply have the obvious thing work without any additional syntax?

Basically what I'd like to happen (whether or not it's possible is another question) is for andadei's example to function as he expected it would.

I haven't been in the code in-depth for a very long time so I actually do not know if that's possible or what we would have to give up in order to have it (for example query caching inside the query object maybe) so that investigation would have to be done before we could make a decision.

@stephenafamo
Copy link
Contributor Author

@stephenafamo stephenafamo commented Jun 18, 2020

Okay. I'll do some research on this later, and see if I can think of a clean way to implement this.

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.