BeforeSelectHook

[KopiasCsaba]

Hi!

I think it would be great, to have a BeforeSelectHook, where we could modify the incoming query.

It would be great for things like enforce deleted_at is null.

What do you think?

Thanks!

[aarondl]

We haven't done this in the past because there was previously no way to disable hook processing for a given query. Now there is so we could consider something like this but I still prefer writing a small helper function to apply the necessary filters despite being easy to miss.

[frederikhors]

@aarondl any news on this?

[aarondl]

@frederikhors Nope. The label "help wanted" implies I'm not going to be taking care of this, and no one else seems to be interested in doing it either! :)

[namco1992]

Hi @aarondl, I created this issue long time back and we end up maintaining our own templates. It seems @frederikhors was facing the same problem here. Would you like to shed light on how would you like to resolve this?

I can think of two solutions:

1. Add support for BeforeSelectHook as this issue mentioned. However, I'm not sure how we can mutate the query in the hooks since the hook func signature is func(context.Context, boil.ContextExecutor, *Model) error.
2. Add a flag to enable the soft delete, it includes adding soft delete func and update selects to exclude soft-deleted records, which is what we are doing in our templates.

I would like to work on this if you would like to share some thoughts. Sqlboiler is widely used in our projects since I first made this decision, and I feel it's not ideal to maintain our own templates as it will get out of sync and even broken in the future. Therefore I propose to help and make it compatible with our needs and probably other people's needs too. Thanks.

[aarondl]

@namco1992 I think that without proper template support you could not do any kind of soft deletion. 1 is definitely out of the picture due to this (given the signature). The queries are also sort of write-once in a way.

I think the only way to accomplish this is exactly what you've mentioned, a soft delete "magic field" much like we have created_at timestamp and updated_at timestamp, you would need deleted bool.

There are quite a few places where selects happen however, so we would need to be cognizant of all of them:

• Regular selects (models.Users())
• Relationship helpers (models.User.Videos())
• Eager loading (models.Users(qm.Load("Videos")))

It's also not 100% clear what we're supposed to do for APIs such as DeleteAll(). Assuming that in a soft-delete world we would want this to soft-delete and not ACTUALLY delete, how do we then delete for real?

It's also true that if we were to implement this functionality as it is a breaking change it would necessitate an enable flag. In the same way that we have --no-auto-timestamps we would need to add a flag for --soft-deletes. Perhaps in v4 we could flip it to --no-soft-deletes.

Thoughts?

[namco1992]

Hi @aarondl, Thank you for your reply. Then I have a proposal:

1. Instead of having deleted bool, I prefer deleted_at timestamp. It is more consistent with the other 2 timestamp fields.
2. In terms of your concern of soft delete and hard delete, I suggest that we keep both. In my opinion, soft delete is not meant to replace the hard delete methods but merely providing another choice as an "add-on". With the flag --soft-deletes=true (or --no-soft-deletes=false), the soft delete methods will be generated with Soft prefix, such as SoftDelete and SoftDeleteAll. In the meantime, all the hard delete methods are generated all the time. (Or we can make the Delete methods as soft delete and create HardDelete methods.) It's up to users to choose which one to use based on their use cases.

[aarondl]

@namco1992 I agree with the deleted_at timestamp. It sparks a question in my mind that I don't know the answer to: Is an index on a bool much more efficient than an index on a timestamp field? Would someone be penalized in performance/storage size for indexing a deleted_at timestamp null vs a deleted bool not null? This has no bearing on what we're going to do - deleted_at is a better general purpose mechanism I think for consistency with the other timestamps but also because it contains more data than a bool.

Thinking about it more it's probably best if we break up the discussion into two pieces of API.

1. Deletions: I think it makes more sense to make existing Delete methods take a soft bool so as to force the user to be aware and always make a conscious choice whether or not they want to be soft deleting, but also make it pretty frictionless. If you create different methods a user may assume "sqlboiler is doing soft-deleting for me" but accidentally pick the method that makes the most sense for their operation DeleteAll() and then they're losing data. Whereas when the compiler complains/his editor tells him he's missing a soft bool in the DeleteAll() method, he's forced to make a choice and this seems like a safer route. What do you think?

2. Selections: How do we allow users to do selects without filtering on soft delete? For example: models.Users().All() will of course bring back all the users, but should that include soft-deleted users or not? I think if you're a user of a soft-deleting system it should probably not bring back any soft deleted users as it is the default action. That decision aside, how do we then indicate that we would like -all- users, soft deleted included?

I think if we can agree on 1, and come up with something for 2 we'll be ready for a PR :)

[namco1992]

Hi @aarondl, I agree on 1, in terms of the second point I think a raw query will do. When you introduce the soft delete into your system, it rarely happens that you want to interact with the deleted records. What do you think?

[aarondl]

@namco1992 I'll accept that for now. It's true no matter what that we would be adding some sort of new API for querying soft-deleted things anyway so we can move forward with adding the normal functionality of never querying soft deleted things. Looking forward to using this PR in my own projects :)

[Hades32]

Is there a reason, why there is no "simple" before select hook? we wanted to use this to trace execution times...

[aarondl]

@Hades32 nope. As you can see above it's just that it's not done. Though you should trace execution times with one of the Go database trace tools more than using sqlboiler's hook system to implement it, it's much more thorough.

[Hades32]

@Hades32 nope. As you can see above it's just that it's not done. Though you should trace execution times with one of the Go database trace tools more than using sqlboiler's hook system to implement it, it's much more thorough.

thanks, I'll check that out first 👍