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

Update utils.py #6933

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update utils.py #6933

wants to merge 3 commits into from

Conversation

mmdsnb
Copy link
Contributor

@mmdsnb mmdsnb commented Mar 5, 2025

feat: Address MySQL limitations in delete operation

Changes Made:

  1. MySQL does not support LIMIT in subqueries within IN clauses:

    • Previous implementation attempted to use a subquery with LIMIT inside an IN clause for deletion operations, which is not supported by MySQL.
    • To resolve this issue, the operation has been split into two steps:
      • First, fetch the IDs of the records to be deleted using a SELECT query.
      • Second, perform the DELETE operation based on these IDs.
  2. MySQL does not support DELETE operations where the filtering condition queries the same table:

    • Originally, the DELETE statement included a WHERE clause that referenced the same table being deleted from, which caused compatibility issues.
    • This has been addressed by first selecting the relevant IDs into a temporary list and then performing the DELETE operation using this list of IDs.

Summary:

To ensure compatibility with MySQL, the code now performs deletions in two steps:

  1. Query the IDs of the records to be deleted.
  2. Use these IDs to execute the DELETE statement.

This change avoids both the limitation of using LIMIT within subqueries in IN clauses and the issue of referencing the same table in the DELETE statement's WHERE clause.

Please review and let me know if there are any further adjustments needed.

Compatibility with MySQL
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 5, 2025
Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mmdsnb

Thanks for this fix.

I'm concerned about how performant this is.

)

# Execute the subquery and fetch the result
result = await session.exec(subquery)
ids_to_delete = [row[0] for row in result]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this works. Shouldn't it be row.id?

Copy link

codspeed-hq bot commented Mar 7, 2025

CodSpeed Performance Report

Merging #6933 will improve performances by 14.07%

Comparing mmdsnb:main (e76189f) with main (a5a8227)

Summary

⚡ 1 improvements
✅ 18 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_build_flow_invalid_job_id 9.1 ms 8 ms +14.07%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants