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

Problem with renaming deleted_at column inside model #2248

Closed
devboev opened this issue Sep 21, 2019 · 4 comments
Closed

Problem with renaming deleted_at column inside model #2248

devboev opened this issue Sep 21, 2019 · 4 comments

Comments

@devboev
Copy link

devboev commented Sep 21, 2019

If you use useSoftDelete and Dateformate as php timestamps inside a model a errornous sql will be created. I created an integer column with default value 0. The created sql from a find request to get the rows from the model is select * from users where <> is null, which results in a empty array.

@MGatner
Copy link
Member

MGatner commented Sep 24, 2019

It looks to me like it is actually choking on escaping your $deletedField. Can you paste or PasteBin the top of your model file?

@devboev
Copy link
Author

devboev commented Sep 24, 2019 via email

@jim-parry jim-parry added this to the 4.0.0-rc.3 milestone Sep 27, 2019
@michalsn
Copy link
Member

I created an integer column with default value 0

The default value for this field supposed to be NULL not 0.

When useSoftDeletes option is enabled the actual query we're using is something like:

SELECT * FROM `table` WHERE `table`.`deleted_at` IS NULL

Maybe we could consider introducing a strictDeleteMode or something? When enabled it would search for 0 for timestamp, or for an appropriate value (0000-00-00 00:00:00 etc) for the date field.

The truth is it could be beneficial for some people with big data sets since it would use less data and index space.

But I don't have a strong opinion about this potential change.

@lonnieezell
Copy link
Member

I hesitate to add yet another setting. I think unless we see this becoming a bigger problem we roll with it the way it currently is, where it checks for null.

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

No branches or pull requests

5 participants