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

Is there some way to get the number of rows deleted by a query builder's delete method? #736

Closed
1 task done
jfly opened this issue Jun 2, 2022 · 7 comments
Closed
1 task done
Labels
enhancement A feature that exists, works as intended but needs to be improved major This issue can only be done in a major release because there are some breaking changes

Comments

@jfly
Copy link

jfly commented Jun 2, 2022

Is your feature request related to a problem?

The query builder docs mention a delete method: https://orm.masoniteproject.com/query-builder#deletes, but I don't see any way of figuring out how many rows were affected by that delete.

What do we currently have to do now?

Right now, I'm turning around and immediately doing a SELECT ROW_COUNT() query, which may be a MySQL specific solution.

Describe the solution you'd like

It would be nice to have a standard way of fetching the number of rows affected by the delete. Perhaps delete() could return the count of deleted rows? I'm not sure if that would count as a breaking change or not. The docs don't seem to mention what delete returns, but the implementation clearly does return... something:

def delete(self, column=None, value=None, query=False):
"""Specify the column and value to delete
or deletes everything based on a previously used where expression.
Keyword Arguments:
column {string} -- The name of the column (default: {None})
value {string|int} -- The value of the column (default: {None})
Returns:
self
"""
model = None
self.set_action("delete")
if self._model:
model = self._model
if column and value:
if isinstance(value, (list, tuple)):
self.where_in(column, value)
else:
self.where(column, value)
if query:
return self
if model and model.is_loaded():
self.where(model.get_primary_key(), model.get_primary_key_value())
self.observe_events(model, "deleting")
result = self.new_connection().query(self.to_qmark(), self._bindings)
if model:
self.observe_events(model, "deleted")
return result
, although the docstring disagrees with the implementation: the docstring says it returns self, when it actually returns the result of self.new_connection().query...

Describe alternatives you've considered

No response

Would this be a breaking change ?

  • Yes

Anything else ?

No response

@jfly jfly added the enhancement A feature that exists, works as intended but needs to be improved label Jun 2, 2022
@girardinsamuel
Copy link
Contributor

I will let @josephmancuso answer this one.

In the meantime, could you move this issue to Masonite ORM repository ? This question belongs there. In the future it will help people with the same questions finding the answer there. 🙏

@jfly
Copy link
Author

jfly commented Jun 3, 2022

Oops, sorry! Reading through https://docs.github.com/en/issues/tracking-your-work-with-issues/transferring-an-issue-to-another-repository, I don't think I have the power to transfer this issue. I can close this and open a new issue of you like. Or perhaps you or another admin could transfer the issue?

@josephmancuso
Copy link
Member

Theres currently no way to do this now but it COULD be possible by getting the values from the databases cursor object

@josephmancuso
Copy link
Member

I would assume this would be a breaking change though if we changed the delete method to return the number of rows affected

@stoutput
Copy link
Contributor

stoutput commented Jun 6, 2022

Yes, for a pymysql connection you could use something like: connection.get_cursor().rowcount

@josephmancuso josephmancuso added the major This issue can only be done in a major release because there are some breaking changes label Jun 25, 2022
@josephmancuso josephmancuso transferred this issue from MasoniteFramework/masonite Jun 25, 2022
@josephmancuso
Copy link
Member

josephmancuso commented Jun 28, 2022

I will work on this one but it won't be available until Masonite ORM 3 around September since this is changing existing functionality in a way that could not be backwards compatable

@josephmancuso
Copy link
Member

This wil also be a bit tricky because the SoftDeleteMixin changes the DELETE to an UPDATE. so will need to figure out how to get the results for an update clause as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature that exists, works as intended but needs to be improved major This issue can only be done in a major release because there are some breaking changes
Projects
None yet
Development

No branches or pull requests

4 participants