Skip to content

Conversation

@ehasrouni
Copy link
Contributor

@ehasrouni ehasrouni commented Sep 12, 2025

Problem: affected_rows always returned 0 for ON DUPLICATE KEY UPDATE operations in Trilogy adapter.

Root Cause: In this PR, we had used raw_connection.affected_rows which returns stale/incorrect values for ON DUPLICATE KEY UPDATE in Trilogy.

The correct values are available in result.affected_rows from the execute result object, so this simply changes the implementation to make use of the result rather than the connection to fetch the affected_rows, super simple change.

This PR:
Switch from raw_connection.affected_rows to result.affected_rows when available
Preserve nil for adapters without support
Adds a test to cover for that edge case
Fix batch accumulation to handle nil values properly

@ehasrouni ehasrouni force-pushed the eh-add-affectors-rows-support-for-on-duplicate-key-update branch 10 times, most recently from e783d14 to 78f85fa Compare September 16, 2025 02:28
@ehasrouni ehasrouni changed the title Eh add affectors rows support for on duplicate key update Fix affected_rows data source for ON DUPLICATE KEY UPDATE operations Sep 18, 2025
@ehasrouni ehasrouni force-pushed the eh-add-affectors-rows-support-for-on-duplicate-key-update branch from 9b6ec27 to 3754da6 Compare September 18, 2025 17:02
@ehasrouni ehasrouni force-pushed the eh-add-affectors-rows-support-for-on-duplicate-key-update branch from 3754da6 to a9ebe39 Compare September 18, 2025 17:04
@ehasrouni ehasrouni marked this pull request as ready for review September 18, 2025 17:06
@ehasrouni
Copy link
Contributor Author

ehasrouni commented Sep 18, 2025

@jkowens hey! this should be the last piece of the puzzle (a small bug fix to what we merged last week). It just adds support for ON DUPLICATE KEY UPDATE operations that were returning 0 with the Trilogy adapter by simply using the result's affected_rows rather than the raw_connection. Business as usual for unsupported adapters.

Thanks a lot! let me know if you have any other concerns

@jkowens jkowens merged commit 85da86e into zdennis:master Sep 19, 2025
29 checks passed
@jkowens
Copy link
Collaborator

jkowens commented Sep 19, 2025

Thanks @ehasrouni 👍

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.

2 participants