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

bugfix: send whole row on UPDATE when there is a BEFORE trigger on table #194

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

francoisp
Copy link

bug fix (and test) to send whole row on update when there's a before trigger.
(weird had to manually create tables in mysql create foreign table does not create a table in mysql?) tested with pg11 and mysql 5.7

…igger. (weird had to manually create tables in mysql create foreign table does not create a table in mysql?) tested with pg11 and mysql 5.7
@surajkharage19
Copy link

Thanks, @francoisp for reporting this. I am able to reproduce the issue at my end. Your changes look good to me, however, I see concerns with respect to row identifier column.

1: If we try to update the row identifier column through BRU trigger then it is not reflecting as new value. Except for that column, all other columns are updated as per their new value assigned in BRU trigger.

2: If we try to update the row identifier column through an update statement (considering that table has BRU trigger) then it is NOT throwing an error, nor updating any of the remaining columns.
e.g:
update mysql_insert_test set c1=100, id =10 where id=3;

In above statement, id is a row identifier column.

For point 1, we can check if the row identifier column value has been changed by before row trigger then we can throw an error by comparing it with old value.

For point 2, throw an error when we find row identifier column in the target list considering that table has before row update trigger. For this, we need to search row identifier column in rte->updatedCols.

I have done these changes in the attached patch, please let me know your feedback.

BRU_trigger_fix.txt

@francoisp
Copy link
Author

@surajkharage19 Very good points! Sorry for the slow reply, I missed this particular message. This does look like fine solutions to these cases, I'll report shortly, I think this covers all angles. Thanks for looking into this.

@francoisp
Copy link
Author

I've updated the PR

@francoisp
Copy link
Author

I've created an new clean pull request

jeevanchalke added a commit that referenced this pull request Oct 5, 2020
Earlier, only columns that are present in the UPDATE query got
updated at the MySQL side.  However, in any column value which is
not present in the UPDATE query but modified in the BEFORE ROW
trigger was not updated at MySQL side.  Fix that by marking all
columns as updatable in such cases.  The fix for this was given
by Francois Payette through pull request #194 on GitHub, which is
further revised by Suraj Kharage.

Also, updating a row-identifier column is not supported.  However,
this was broken when the BR update trigger is trying to update that.
Fix that by comparing old and new value for the row-identifier column
and throwing an error if they are not the same.

Reported on GitHub through issues #193 by Francois Payette.

FDW-193, Suraj Kharage, reviewed by Vaibhav Dalvi and me.
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