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

Fix incorrect deleted row count #26524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Francisco-Morao
Copy link

The deleted row count was incorrect when using the USING clause. The problem was caused by the target_tuple_fetched variable always being set to true, preventing rows_affected_count from updating correctly and skipping the break statement. As a result, es_processed was not incremented as expected.

The fix involves introducing an auxiliary variable (rows_affected_aux) to store the number of rows affected by YBCExecWriteStmt. Then, this value is accumulated into rows_affected_count, ensuring accurate tracking.

Fix #25305

Summary:
This commit fixes an issue where the deleted row count was incorrect
when using the USING clause. The problem was caused by the
target_tuple_fetched variable always being set to true, preventing
rows_affected_count from updating correctly and skipping the break
statement. As a result,es_processed was not incremented as expected.

Changes:
1. Fixed the incorrect row count issue by ensuring target_tuple_fetched
is set correctly.
2.Added a Java test (TestPgDelete) to verify the fix.

Fixes yugabyte#25305

Test Plan:
Added a Java test (TestPgDelete), which now passes successfully.
@CLAassistant
Copy link

CLAassistant commented Mar 24, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Mar 24, 2025

Deploy Preview for infallible-bardeen-164bc9 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 607c084
🔍 Latest deploy log https://app.netlify.com/sites/infallible-bardeen-164bc9/deploys/67e1e12dd0206f000870b5eb
😎 Deploy Preview https://deploy-preview-26524--infallible-bardeen-164bc9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@karthik-ramanathan-3006
Copy link
Contributor

@Francisco-Morao - Thank you for contributing to the yugabyte-db repository!

UPDATE statements with a FROM clause also suffer from the same problem - returning inflated counts when a JOIN producing duplicate rows is used to feed the Update/Delete node.

Ideally, we'd like to use the same fix in both cases. Could you please try your fix with UPDATEs?
I suspect that this might not work as rows_affected_count would no longer return 0 for the duplicate row.

@Francisco-Morao
Copy link
Author

@Francisco-Morao - Thank you for contributing to the yugabyte-db repository!

UPDATE statements with a FROM clause also suffer from the same problem - returning inflated counts when a JOIN producing duplicate rows is used to feed the Update/Delete node.

Ideally, we'd like to use the same fix in both cases. Could you please try your fix with UPDATEs? I suspect that this might not work as rows_affected_count would no longer return 0 for the duplicate row.

I just tried to make the change, but it did not work. The problem is most likely in YBCExecuteUpdate, as row_found always returns 1 so true regardless of the conditions. Investigating and solving this bug would require more time. Maybe we should open a new issue for this if one does not already exist.

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

Successfully merging this pull request may close these issues.

[YSQL] Deleted row count reported is incorrect when using DELETE with USING clause
3 participants