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.

Sign up for GitHub

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

Jump to bottom

SingleStore does not allow a order by in a delete query #84

Open
Cosnavel opened this issue May 3, 2024 · 3 comments · May be fixed by #85
Open

SingleStore does not allow a order by in a delete query #84

Cosnavel opened this issue May 3, 2024 · 3 comments · May be fixed by #85
Assignees
@AdalbertMemSQL

Comments

@Cosnavel
Copy link

Cosnavel commented May 3, 2024 •
edited
Loading

We are in the process of migrating to Singlestore and have encountered a compatibility issue with Laravel's Database Notifications.

The issue arises from the HasDatabaseNotifications trait, which defines a relationship that automatically applies a latest sorting order by default. This sorting behavior is intended for notification retrieval but also inadvertently affects deletion operations. While this automatically generated query functions correctly in MySQL and PostgreSQL, it appears that Singlestore does not support this syntax.

HasDatabaseNotification Trait and Relation 
<?php

namespace Illuminate\Notifications;

trait HasDatabaseNotifications
{
    /**
     * Get the entity's notifications.
     *
     * @return \Illuminate\Database\Eloquent\Relations\MorphMany
     */
    public function notifications()
    {
        return $this->morphMany(DatabaseNotification::class, 'notifiable')->latest();
    }
Example of a produced query when trying to delete a notification via relation and the corresponding error from Singlestore 
delete from
  `notifications`
where
  `notifications`.`notifiable_type` = App \ Models \ User
  and `notifications`.`notifiable_id` = 218
  and `notifications`.`notifiable_id` is not null
  and JSON_EXTRACT_STRING(data, 'format') = filament
  and `id` = 9bf43012 - ecff - 4ef1 - afb2 - 26ee0218109f
order by
  `created_at` desc
SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'order by `created_at` desc' at line 1

@carlsverre the behaviour should be documented in Singlestores doc. In the example is a order by in a subquery but no mention of the not allowed behaviour.

Reference: https://docs.singlestore.com/cloud/reference/sql-reference/data-manipulation-language-dml/delete/#example

@olliescase
@Cosnavel Cosnavel linked a pull request May 3, 2024 that will close this issue
Open
@AdalbertMemSQL
Copy link
Collaborator

Hey @Cosnavel
Thanks for pointing out this incompatibility.

With your changes, the connector may produce an incorrect result.
For example, when someone wants to delete a single row with the latest timestamp (without order by, it will delete the random row, which will be very unexpected).

I prefer to throw an error in this case by default and add a special flag ignore_order_by_in_delete which can be enabled in the case of migrations when the user knows that order by can be safely ignored.

What do you think about this?

@Cosnavel
Copy link
Author

Cosnavel commented May 3, 2024

Hey @AdalbertMemSQL,
would also be an idea to overwrite the entire handling of the order by in deletes and inject it as a subquery. Would that be possible?

We also encountered several errors because of subqueries like this so in a bit concerned.

General error: 1749 Feature 'Correlated subselect that can not be transformed and does not match on shard keys' is not supported by SingleStore Distributed.

@AdalbertMemSQL
Copy link
Collaborator

Hmm....
I think that in some cases (when the table has a unique key) it should be possible.
But I'm not sure about the performance of such queries.
I need to investigate it deeper.
Are you blocked by this feature? I can work on enabling a possibility to ignore order by in deletes and then work on the investigation of implementing this functionality as subselects.

Can you open a separate issue regarding subqueries with examples of queries that caused this error?

@AdalbertMemSQL AdalbertMemSQL self-assigned this May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AdalbertMemSQL AdalbertMemSQL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ignore order by in delete queries Kettner-Edelmetalle/singlestoredb-laravel-driver
2 participants
@Cosnavel @AdalbertMemSQL