-
-
Notifications
You must be signed in to change notification settings - Fork 265
New setting UpdateOverwriteMode : defines how UPDATE should handle the case when a record was updated by trigger. #8957
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
base: v5.0-release
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,7 +174,7 @@ static void protect_system_table_delupd(thread_db* tdbb, const jrd_rel* relation | |
| bool force_flag = false); | ||
| static void purge(thread_db*, record_param*); | ||
| static void replace_record(thread_db*, record_param*, PageStack*, const jrd_tra*); | ||
| static void refresh_fk_fields(thread_db*, Record*, record_param*, record_param*); | ||
| static void refresh_changed_fields(thread_db*, Record*, record_param*, record_param*); | ||
| static SSHORT set_metadata_id(thread_db*, Record*, USHORT, drq_type_t, const char*); | ||
| static void set_nbackup_id(thread_db*, Record*, USHORT, drq_type_t, const char*); | ||
| static void set_owner_name(thread_db*, Record*, USHORT); | ||
|
|
@@ -3329,7 +3329,7 @@ bool VIO_modify(thread_db* tdbb, record_param* org_rpb, record_param* new_rpb, j | |
| fb_assert(!(org_rpb->rpb_runtime_flags & RPB_undo_read)); | ||
|
|
||
| if (undo_read) | ||
| refresh_fk_fields(tdbb, old_record, org_rpb, new_rpb); | ||
| refresh_changed_fields(tdbb, old_record, org_rpb, new_rpb); | ||
| } | ||
|
|
||
| // If we're the system transaction, modify stuff in place. This saves | ||
|
|
@@ -6605,43 +6605,43 @@ static void replace_record(thread_db* tdbb, | |
| } | ||
|
|
||
|
|
||
| static void refresh_fk_fields(thread_db* tdbb, Record* old_rec, record_param* cur_rpb, | ||
| static void refresh_changed_fields(thread_db* tdbb, Record* old_rec, record_param* cur_rpb, | ||
| record_param* new_rpb) | ||
| { | ||
| /************************************** | ||
| * | ||
| * r e f r e s h _ f k _ f i e l d s | ||
| * r e f r e s h _ c h a n g e d _ f i e l d s | ||
| * | ||
| ************************************** | ||
| * | ||
| * Functional description | ||
| * Update new_rpb with foreign key fields values changed by cascade triggers. | ||
| * Consider self-referenced foreign keys only. | ||
| * Also, if UpdateOverwriteMode is set to 1, raise error when non self-referenced | ||
| * foreign key fields were changed by user triggers. | ||
| * | ||
| * old_rec - old record before modify | ||
| * cur_rpb - just read record with possibly changed FK fields | ||
| * cur_rpb - just read record with possibly changed fields | ||
| * new_rpb - new record evaluated by modify statement and before-triggers | ||
| * | ||
| **************************************/ | ||
| const Database* dbb = tdbb->getDatabase(); | ||
| const auto overwriteMode = dbb->dbb_config->getUpdateOverwriteMode(); | ||
|
|
||
| jrd_rel* relation = cur_rpb->rpb_relation; | ||
|
|
||
| MET_scan_partners(tdbb, relation); | ||
|
|
||
| if (!(relation->rel_foreign_refs.frgn_relations)) | ||
| return; | ||
|
|
||
| const FB_SIZE_T frgnCount = relation->rel_foreign_refs.frgn_relations->count(); | ||
| if (!frgnCount) | ||
| return; | ||
| const FB_SIZE_T frgnCount = relation->rel_foreign_refs.frgn_relations ? | ||
| relation->rel_foreign_refs.frgn_relations->count() : 0; | ||
|
|
||
| RelationPages* relPages = cur_rpb->rpb_relation->getPages(tdbb); | ||
|
|
||
| // Collect all fields of all foreign keys | ||
| // Collect all fields of self-referenced foreign keys | ||
| SortedArray<int, InlineStorage<int, 16> > fields; | ||
|
|
||
| for (FB_SIZE_T i = 0; i < frgnCount; i++) | ||
| { | ||
| // We need self-referenced FK's only | ||
| if ((*relation->rel_foreign_refs.frgn_relations)[i] == relation->rel_id) | ||
| { | ||
| index_desc idx; | ||
|
|
@@ -6663,26 +6663,63 @@ static void refresh_fk_fields(thread_db* tdbb, Record* old_rec, record_param* cu | |
| } | ||
|
|
||
| if (fields.isEmpty()) | ||
| return; | ||
| { | ||
| if (overwriteMode == 0) | ||
| return; | ||
|
|
||
| DSC desc1, desc2; | ||
| for (FB_SIZE_T idx = 0; idx < fields.getCount(); idx++) | ||
| if (cur_rpb->rpb_record->getFormat()->fmt_version == old_rec->getFormat()->fmt_version) | ||
| { | ||
| if (memcmp(cur_rpb->rpb_address, old_rec->getData(), cur_rpb->rpb_length) == 0) | ||
| return; | ||
|
|
||
| fb_assert(overwriteMode == 1); | ||
| ERR_post(Arg::Gds(isc_update_overwrite_trigger)); // UPDATE will overwrite changes made by trigger | ||
| } | ||
| // Else compare field-by-field | ||
| } | ||
|
|
||
| for (FB_SIZE_T fld = 0, frn = 0; fld < relation->rel_current_format->fmt_count; fld++) | ||
| { | ||
| // Detect if user changed FK field by himself. | ||
| const int fld = fields[idx]; | ||
| const bool flag_old = EVL_field(relation, old_rec, fld, &desc1); | ||
| const bool flag_new = EVL_field(relation, new_rpb->rpb_record, fld, &desc2); | ||
| dsc dsc_old; | ||
| const bool flag_old = EVL_field(relation, old_rec, fld, &dsc_old); | ||
|
|
||
| // If field was not changed by user - pick up possible modification by | ||
| // system cascade trigger | ||
| if (flag_old == flag_new && | ||
| (!flag_old || (flag_old && !MOV_compare(tdbb, &desc1, &desc2)))) | ||
| const bool is_fk = (frn < fields.getCount() && fields[frn] == fld); | ||
| if (!is_fk) | ||
| { | ||
| const bool flag_tmp = EVL_field(relation, cur_rpb->rpb_record, fld, &desc1); | ||
| if (flag_tmp) | ||
| MOV_move(tdbb, &desc1, &desc2); | ||
| else | ||
| new_rpb->rpb_record->setNull(fld); | ||
| if (overwriteMode == 0) | ||
| continue; | ||
|
|
||
| dsc dsc_cur; | ||
| const bool flag_cur = EVL_field(relation, cur_rpb->rpb_record, fld, &dsc_cur); | ||
|
|
||
| // Check if current record differs from old record | ||
| if ((flag_cur != flag_old) || | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the field didn't exist in the old format but exists in the new format and gets some value updated by a trigger (e.g. because the new field is
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If any field of current record was changed by trigger, that fired for another record - yes, this is error. |
||
| (flag_cur && flag_old && MOV_compare(tdbb, &dsc_old, &dsc_cur) != 0)) | ||
| { | ||
| // Record was modified by trigger. | ||
| fb_assert(overwriteMode == 1); | ||
| ERR_post(Arg::Gds(isc_update_overwrite_trigger)); // UPDATE will overwrite changes made by trigger | ||
| } | ||
| } | ||
| else | ||
| { | ||
| dsc dsc_new; | ||
| const bool flag_new = EVL_field(relation, new_rpb->rpb_record, fld, &dsc_new); | ||
|
|
||
| // If field was not changed by user - pick up possible modification by | ||
| // system cascade trigger | ||
| if (flag_old == flag_new && | ||
| (!flag_old || (flag_old && !MOV_compare(tdbb, &dsc_old, &dsc_new)))) | ||
| { | ||
| dsc dsc_cur; | ||
| const bool flag_cur = EVL_field(relation, cur_rpb->rpb_record, fld, &dsc_cur); | ||
| if (flag_cur) | ||
| MOV_move(tdbb, &dsc_cur, &dsc_new); | ||
| else | ||
| new_rpb->rpb_record->setNull(fld); | ||
| } | ||
|
|
||
| frn++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
integerand0/1instead ofbooleanandfalse/true? And rename to something likeUpdateOverwriteStrictModeorRestrictUpdateOverwrite?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I thought about 3rd value - to issue warning, not error. But it was not implemented as I found it unpractical, iirc.
As for the setting name - I can agree with anything reasonable ;) Lets decide if we need such changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I going to add forgoten note that this setting is temporary and could be removed in future versions.