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

Bug: DELETE gives strange Cannot perform multiplication error #3236

Closed
2 tasks done
AlexFrid opened this issue Dec 29, 2023 · 7 comments
Closed
2 tasks done

Bug: DELETE gives strange Cannot perform multiplication error #3236

AlexFrid opened this issue Dec 29, 2023 · 7 comments
Assignees
Labels
noissue This can already be done topic:surrealql This is related to the SurrealQL query language

Comments

@AlexFrid
Copy link

Describe the bug

When importing the demo dataset through the CLI there is a strange error that appears when trying to delete some records.

For example: delete product:00a2xf120s1z7q52f69z gives the error:

There was a problem with the database: Cannot perform multiplication with '16944.01f' and 'NONE'

When looking at the record you find that the price field is 16944.01

This only happens when you import a lot of records through the CLI, when you isolate just the problem records and import them it works without issues.

Same goes for just for using the /sql endpoint, it works fine.

This only effects certain records, for example delete product:00b2as236k7h9v33a95p; works as expected, but there is no easily discernible difference that would show we one works and the other doesn't.

Steps to reproduce

Import the demo dataset and run this query:

delete product:00a2xf120s1z7q52f69z

Expected behaviour

Expected the record to be deleted without error.

SurrealDB version

1.0.0+20230913.54aedcd for macos on aarch64

Contact Details

alexander.fridriksson@surrealdb.com

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • I agree to follow this project's Code of Conduct
@AlexFrid AlexFrid added bug Something isn't working triage This issue is new labels Dec 29, 2023
@LucyEgan
Copy link

Ive noticed errors during deletes because of futures/events that are getting fired that are now invalid based on the calculation of the deleted data.
Mine was a event that accessed $after.field, and that field was a future that became "broken" from the DELETE, so it was a underlying query not the actual DELETE that was erroring.

Is this related to those records that cause issues? (not got access to surreal to test currently)

@phughk
Copy link
Contributor

phughk commented Jan 5, 2024

This falls out of my scope, but being aware of how many times I have tagged - assigning to myself. Going to bring up in next eng meet about how we spread and prioritise workload.

For the ticket: It would seem that there is processing on the document, but I don't understand how that would happen since deletes don't really process the document. @DelSkayn if you have any input that would be appreciated!

@phughk phughk self-assigned this Jan 5, 2024
@phughk phughk removed the triage This issue is new label Jan 5, 2024
@DelSkayn
Copy link
Member

I am looking into this but I am currently getting a different error. I downloaded the surreal deal dataset and imported it successfully. I am however facing an issue the order relation. All orders are related via a order record id which consists of a country and seemingly a datetime. However the datetime is in the form of 2023-01-04T03:55:50.718612. This is an invalid datetime as it is missing a timezone. So the current error I am getting is that 2023-01-04T03:55:50.718612 can't be converted to a datetime.

@AlexFrid
Copy link
Author

So the current error I am getting is that 2023-01-04T03:55:50.718612 can't be converted to a datetime.

Interesting, once I upgraded to v1.1 I also get that error 🤔

@DelSkayn
Copy link
Member

The datetime error seems to be a different error: Previously there was some dubious behavior when parsing date-times. If only a part of the string matched a datetime it would still parse the datetime. so 2012-04-23blablabla would parse the 2012-04-23 part and ignore the blablabla. So with 2012-04-23T18:25:43.5110000 it fails to parse the full datetime since the time zone is missing and therefore would only parse 2012-04-23 . Now it errors instead if there is still some text left which causes the error I found.

Updating the dataset to have the correct datetimes makes the multiplication error return.

@kearfy kearfy added the topic:surrealql This is related to the SurrealQL query language label Jan 16, 2024
@DelSkayn DelSkayn added noissue This can already be done and removed bug Something isn't working labels Feb 6, 2024
@DelSkayn
Copy link
Member

DelSkayn commented Feb 6, 2024

I believe this issue can be closed. The problem seemed to be inside one of the queries where it would try to multiply a number times a null because of a missing else branch so the error message was actually correct.

We still have a problem that the error message here is not that useful but that should probably be a different issue.

@DelSkayn
Copy link
Member

DelSkayn commented Feb 6, 2024

For those interested what the actual problem was, in the sureal-deal dataset there was the query:

DEFINE TABLE daily_sales SCHEMALESS AS 
    SELECT count() AS number_of_orders, 
        time::format(<datetime> order_date, '%Y-%m-%d') AS day, 
        math::sum((price * IF discount = NONE THEN 1 END) * quantity) AS sum_sales, 
        currency FROM order GROUP BY day, currency;

Which should have been:

DEFINE TABLE daily_sales SCHEMALESS AS 
    SELECT count() AS number_of_orders, 
        time::format(<datetime> order_date, '%Y-%m-%d') AS day, 
        math::sum((price * IF discount = NONE THEN 1 ELSE discount END) * quantity) AS sum_sales, 
        currency FROM order GROUP BY day, currency;

@AlexFrid AlexFrid closed this as completed Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noissue This can already be done topic:surrealql This is related to the SurrealQL query language
Projects
None yet
Development

No branches or pull requests

5 participants