From 542ac86bf9bb9730cdcf4365cd95f4b6dfd81e52 Mon Sep 17 00:00:00 2001 From: dimitr Date: Fri, 10 Jan 2014 09:53:32 +0000 Subject: [PATCH] Backported CORE-3881: Extend the error reported for index/constraint violations to include the problematic key value. --- src/jrd/btr.cpp | 207 +++++++++++++++++++++++++++++++++++++++++- src/jrd/btr.h | 32 +++++++ src/jrd/err.cpp | 73 --------------- src/jrd/err_proto.h | 1 - src/jrd/exe.cpp | 43 +-------- src/jrd/idx.cpp | 216 ++++++++++++++++++++++++-------------------- src/jrd/idx_proto.h | 13 +-- src/jrd/nav.cpp | 5 +- 8 files changed, 365 insertions(+), 225 deletions(-) diff --git a/src/jrd/btr.cpp b/src/jrd/btr.cpp index 5693d44ef0..80cdf35bf4 100644 --- a/src/jrd/btr.cpp +++ b/src/jrd/btr.cpp @@ -201,6 +201,7 @@ static INT64_KEY make_int64_key(SINT64, SSHORT); #ifdef DEBUG_INDEXKEY static void print_int64_key(SINT64, SSHORT, INT64_KEY); #endif +static string print_key(thread_db*, jrd_rel*, index_desc*, Record*); static contents remove_node(thread_db*, index_insertion*, WIN*); static contents remove_leaf_node(thread_db*, index_insertion*, WIN*); static bool scan(thread_db*, UCHAR*, RecordBitmap**, RecordBitmap*, index_desc*, @@ -211,6 +212,8 @@ static void checkForLowerKeySkip(bool&, const bool, const IndexNode&, const temp const index_desc&, const IndexRetrieval*); +// BtrPageLock class + BtrPageGCLock::BtrPageGCLock(thread_db* tdbb) { Database* dbb = tdbb->getDatabase(); @@ -260,6 +263,81 @@ bool BtrPageGCLock::isPageGCAllowed(thread_db* tdbb, const PageNumber& page) } +// IndexErrorContext class + +void IndexErrorContext::raise(thread_db* tdbb, idx_e result, Record* record) +{ + fb_assert(result != idx_e_ok); + + if (result == idx_e_conversion) + ERR_punt(); + + const MetaName& relationName = isLocationDefined ? m_location.relation->rel_name : m_relation->rel_name; + const USHORT indexId = isLocationDefined ? m_location.indexId : m_index->idx_id; + + MetaName indexName(m_indexName), constraintName; + + if (indexName.isEmpty()) + MET_lookup_index(tdbb, indexName, relationName, indexId + 1); + + if (indexName.hasData()) + MET_lookup_cnstrt_for_index(tdbb, constraintName, indexName); + else + indexName = "***unknown***"; + + const bool haveConstraint = constraintName.hasData(); + + if (!haveConstraint) + constraintName = "***unknown***"; + + switch (result) + { + case idx_e_keytoobig: + ERR_post_nothrow(Arg::Gds(isc_imp_exc) << + Arg::Gds(isc_keytoobig) << Arg::Str(indexName)); + break; + + case idx_e_foreign_target_doesnt_exist: + ERR_post_nothrow(Arg::Gds(isc_foreign_key) << + Arg::Str(constraintName) << Arg::Str(relationName) << + Arg::Gds(isc_foreign_key_target_doesnt_exist)); + break; + + case idx_e_foreign_references_present: + ERR_post_nothrow(Arg::Gds(isc_foreign_key) << + Arg::Str(constraintName) << Arg::Str(relationName) << + Arg::Gds(isc_foreign_key_references_present)); + break; + + case idx_e_duplicate: + if (haveConstraint) + { + ERR_post_nothrow(Arg::Gds(isc_unique_key_violation) << + Arg::Str(constraintName) << Arg::Str(relationName)); + } + else + ERR_post_nothrow(Arg::Gds(isc_no_dup) << Arg::Str(indexName)); + break; + + default: + fb_assert(false); + } + + if (record) + { + const string keyString = print_key(tdbb, m_relation, m_index, record); + if (keyString.hasData()) + { + string errorMsg; + errorMsg.printf("Problematic key value is %s", keyString.c_str()); + ERR_post_nothrow(Arg::Gds(isc_random) << Arg::Str(errorMsg)); + } + } + + ERR_punt(); +} + + USHORT BTR_all(thread_db* tdbb, jrd_rel* relation, IndexDescAlloc** csb_idx, @@ -881,8 +959,11 @@ btree_page* BTR_find_page(thread_db* tdbb, } } - if (errorCode != idx_e_ok) { - ERR_duplicate_error(errorCode, retrieval->irb_relation, retrieval->irb_index); + if (errorCode != idx_e_ok) + { + index_desc temp_idx = retrieval->irb_desc; // to avoid constness issues + IndexErrorContext context(retrieval->irb_relation, &temp_idx); + context.raise(tdbb, errorCode, NULL); } } @@ -6568,6 +6649,128 @@ static void print_int64_key(SINT64 value, SSHORT scale, INT64_KEY key) #endif // DEBUG_INDEXKEY +string print_key(thread_db* tdbb, jrd_rel* relation, index_desc* idx, Record* record) +{ +/************************************** + * + * p r i n t _ k e y + * + ************************************** + * + * Functional description + * Convert index key into textual representation. + * + **************************************/ + fb_assert(relation && idx && record); + + if (!(relation->rel_flags & REL_scanned) || + (relation->rel_flags & REL_being_scanned)) + { + MET_scan_relation(tdbb, relation); + } + + class Printer + { + public: + explicit Printer(const dsc* desc) + { + const int MAX_KEY_STRING_LEN = 250; + const char* const NULL_KEY_STRING = "NULL"; + + if (!desc) + { + value = NULL_KEY_STRING; + return; + } + + fb_assert(!desc->isBlob()); + + char temp[BUFFER_TINY]; + const char* str = NULL; + const int len = MOV_make_string(desc, ttype_dynamic, &str, + (vary*) temp, sizeof(temp)); + + value.assign(str, len); + + if (DTYPE_IS_TEXT(desc->dsc_dtype) || DTYPE_IS_DATE(desc->dsc_dtype)) + { + if (DTYPE_IS_TEXT(desc->dsc_dtype) && desc->dsc_sub_type == ttype_binary) + { + string hex; + char* s = hex.getBuffer(2 * len); + for (int i = 0; i < len; i++) + { + sprintf(s, "%02X", (int) (unsigned char) str[i]); + s += 2; + } + value = "x'" + hex + "'"; + } + else + { + value = "'" + value + "'"; + } + } + + if (value.length() > MAX_KEY_STRING_LEN) + { + value.resize(MAX_KEY_STRING_LEN); + value += "..."; + } + } + + const string& get() const + { + return value; + } + + private: + string value; + }; + + string key, value; + + try + { + if (idx->idx_flags & idx_expressn) + { + bool notNull = false; + const dsc* const desc = BTR_eval_expression(tdbb, idx, record, notNull); + value = Printer(notNull ? desc : NULL).get(); + key += " = " + value; + } + else + { + for (USHORT i = 0; i < idx->idx_count; i++) + { + const USHORT field_id = idx->idx_rpt[i].idx_field; + const jrd_fld* const field = MET_get_field(relation, field_id); + + if (field) + value.printf("\"%s\"", field->fld_name.c_str()); + else + value.printf("", field_id); + + key += value; + + dsc desc; + const bool notNull = EVL_field(relation, record, field_id, &desc); + value = Printer(notNull ? &desc : NULL).get(); + key += " = " + value; + + if (i < idx->idx_count - 1) + key += ", "; + } + } + } + catch (const Exception&) + { + return ""; + } + + return "(" + key + ")"; +} + + static contents remove_node(thread_db* tdbb, index_insertion* insertion, WIN* window) { /************************************** diff --git a/src/jrd/btr.h b/src/jrd/btr.h index f9a307959f..e90732a539 100644 --- a/src/jrd/btr.h +++ b/src/jrd/btr.h @@ -253,6 +253,38 @@ public: }; +// Class used to report any index related errors + +class IndexErrorContext +{ + struct Location + { + jrd_rel* relation; + USHORT indexId; + }; + +public: + IndexErrorContext(jrd_rel* relation, index_desc* index, const char* indexName = NULL) + : m_relation(relation), m_index(index), m_indexName(indexName), isLocationDefined(false) + {} + + void setErrorLocation(jrd_rel* relation, USHORT indexId) + { + isLocationDefined = true; + m_location.relation = relation; + m_location.indexId = indexId; + } + + void raise(thread_db*, idx_e, Record*); + +private: + jrd_rel* const m_relation; + index_desc* const m_index; + const char* const m_indexName; + Location m_location; + bool isLocationDefined; +}; + } //namespace Jrd diff --git a/src/jrd/err.cpp b/src/jrd/err.cpp index 5abc266604..60d4a0261b 100644 --- a/src/jrd/err.cpp +++ b/src/jrd/err.cpp @@ -119,79 +119,6 @@ void ERR_corrupt(int number) } -void ERR_duplicate_error(idx_e code, const jrd_rel* relation, USHORT index_number, const TEXT* idx_name) -{ -/************************************** - * - * E R R _ d u p l i c a t e _ e r r o r - * - ************************************** - * - * Functional description - * Duplicate error during index update. - * - **************************************/ - thread_db* tdbb = JRD_get_thread_data(); - - if (code == idx_e_conversion) - ERR_punt(); - - MetaName index, constraint; - if (!idx_name) - MET_lookup_index(tdbb, index, relation->rel_name, index_number + 1); - else - index = idx_name; - - bool haveConstraint = true; - if (index.hasData()) - { - MET_lookup_cnstrt_for_index(tdbb, constraint, index); - if (constraint.isEmpty()) - { - haveConstraint = false; - constraint = "***unknown***"; - } - } - else - { - haveConstraint = false; - index = constraint = "***unknown***"; - } - - switch (code) - { - case idx_e_keytoobig: - ERR_post(Arg::Gds(isc_imp_exc) << - Arg::Gds(isc_keytoobig) << Arg::Str(index)); - break; - - case idx_e_foreign_target_doesnt_exist: - ERR_post(Arg::Gds(isc_foreign_key) << Arg::Str(constraint) << - Arg::Str(relation->rel_name) << - Arg::Gds(isc_foreign_key_target_doesnt_exist)); - break; - - case idx_e_foreign_references_present: - ERR_post(Arg::Gds(isc_foreign_key) << Arg::Str(constraint) << - Arg::Str(relation->rel_name) << - Arg::Gds(isc_foreign_key_references_present)); - break; - - case idx_e_duplicate: - if (haveConstraint) - { - ERR_post(Arg::Gds(isc_unique_key_violation) << Arg::Str(constraint) << - Arg::Str(relation->rel_name)); - } - else - ERR_post(Arg::Gds(isc_no_dup) << Arg::Str(index)); - - default: - fb_assert(false); - } -} - - void ERR_error(int number) { /************************************** diff --git a/src/jrd/err_proto.h b/src/jrd/err_proto.h index 1859493256..4f9c89f7cf 100644 --- a/src/jrd/err_proto.h +++ b/src/jrd/err_proto.h @@ -52,7 +52,6 @@ void ERR_assert(const TEXT*, int); void ERR_bugcheck(int, const TEXT* = NULL, int = 0); void ERR_bugcheck_msg(const TEXT*); void ERR_corrupt(int); -void ERR_duplicate_error(Jrd::idx_e, const Jrd::jrd_rel*, USHORT, const TEXT* = NULL); void ERR_error(int); void ERR_error_msg(const TEXT*); void ERR_post(const Firebird::Arg::StatusVector& v); diff --git a/src/jrd/exe.cpp b/src/jrd/exe.cpp index 5a1db1c42d..ca45e36fa0 100644 --- a/src/jrd/exe.cpp +++ b/src/jrd/exe.cpp @@ -1313,16 +1313,7 @@ static jrd_nod* erase(thread_db* tdbb, jrd_nod* node, SSHORT which_trig) can be implemented as post_erase triggers */ if (!relation->rel_file && !relation->rel_view_rse && !relation->isVirtual()) - { - jrd_rel* bad_relation = 0; - USHORT bad_index; - - const idx_e error_code = IDX_erase(tdbb, rpb, transaction, &bad_relation, &bad_index); - - if (error_code) { - ERR_duplicate_error(error_code, bad_relation, bad_index); - } - } + IDX_erase(tdbb, rpb, transaction); /* CVC: Increment the counter only if we called VIO/EXT_erase() and we were successful. */ @@ -3031,16 +3022,8 @@ static jrd_nod* modify(thread_db* tdbb, jrd_nod* node, SSHORT which_trig) } else if (!relation->rel_view_rse) { - USHORT bad_index; - jrd_rel* bad_relation = 0; - VIO_modify(tdbb, org_rpb, new_rpb, transaction); - const idx_e error_code = - IDX_modify(tdbb, org_rpb, new_rpb, transaction, &bad_relation, &bad_index); - - if (error_code) { - ERR_duplicate_error(error_code, bad_relation, bad_index); - } + IDX_modify(tdbb, org_rpb, new_rpb, transaction); } new_rpb->rpb_number = org_rpb->rpb_number; @@ -3059,18 +3042,7 @@ static jrd_nod* modify(thread_db* tdbb, jrd_nod* node, SSHORT which_trig) which can be implemented as post_erase triggers */ if (!relation->rel_file && !relation->rel_view_rse && !relation->isVirtual()) - { - USHORT bad_index; - jrd_rel* bad_relation = 0; - - const idx_e error_code = - IDX_modify_check_constraints(tdbb, org_rpb, new_rpb, transaction, - &bad_relation, &bad_index); - - if (error_code) { - ERR_duplicate_error(error_code, bad_relation, bad_index); - } - } + IDX_modify_check_constraints(tdbb, org_rpb, new_rpb, transaction); if (transaction != dbb->dbb_sys_trans) { --transaction->tra_save_point->sav_verb_count; @@ -3825,15 +3797,8 @@ static jrd_nod* store(thread_db* tdbb, jrd_nod* node, SSHORT which_trig) } else if (!relation->rel_view_rse) { - USHORT bad_index; - jrd_rel* bad_relation = 0; - VIO_store(tdbb, rpb, transaction); - const idx_e error_code = IDX_store(tdbb, rpb, transaction, &bad_relation, &bad_index); - - if (error_code) { - ERR_duplicate_error(error_code, bad_relation, bad_index); - } + IDX_store(tdbb, rpb, transaction); } rpb->rpb_number.setValid(true); diff --git a/src/jrd/idx.cpp b/src/jrd/idx.cpp index 2a75394b01..cc6c7e1ae8 100644 --- a/src/jrd/idx.cpp +++ b/src/jrd/idx.cpp @@ -75,17 +75,18 @@ using namespace Firebird; struct index_fast_load { + SINT64 ifl_dup_recno; SLONG ifl_duplicates; USHORT ifl_key_length; }; static idx_e check_duplicates(thread_db*, Record*, index_desc*, index_insertion*, jrd_rel*); -static idx_e check_foreign_key(thread_db*, Record*, jrd_rel*, jrd_tra*, index_desc*, jrd_rel**, USHORT *); +static idx_e check_foreign_key(thread_db*, Record*, jrd_rel*, jrd_tra*, index_desc*, IndexErrorContext&); static idx_e check_partner_index(thread_db*, jrd_rel*, Record*, jrd_tra*, index_desc*, jrd_rel*, USHORT); static bool duplicate_key(const UCHAR*, const UCHAR*, void*); static PageNumber get_root_page(thread_db*, jrd_rel*); static int index_block_flush(void*); -static idx_e insert_key(thread_db*, jrd_rel*, Record*, jrd_tra*, WIN *, index_insertion*, jrd_rel**, USHORT *); +static idx_e insert_key(thread_db*, jrd_rel*, Record*, jrd_tra*, WIN *, index_insertion*, IndexErrorContext&); static bool key_equal(const temporary_key*, const temporary_key*); static void release_index_block(thread_db*, IndexBlock*); static void signal_index_deletion(thread_db*, jrd_rel*, USHORT); @@ -290,6 +291,7 @@ void IDX_create_index(thread_db* tdbb, const UCHAR pad = isDescending ? -1 : 0; index_fast_load ifl_data; + ifl_data.ifl_dup_recno = -1; ifl_data.ifl_duplicates = 0; ifl_data.ifl_key_length = key_length; @@ -346,6 +348,8 @@ void IDX_create_index(thread_db* tdbb, } } + IndexErrorContext context(relation, idx, index_name); + // Loop thru the relation computing index keys. If there are old versions, find them, too. temporary_key key; while (DPM_next(tdbb, &primary, LCK_read, @@ -442,7 +446,7 @@ void IDX_create_index(thread_db* tdbb, if (primary.getWindow(tdbb).win_flags & WIN_large_scan) --relation->rel_scan_count; - ERR_duplicate_error(result, relation, idx->idx_id, index_name); + context.raise(tdbb, result, record); } if (key.key_length > key_length) @@ -454,7 +458,8 @@ void IDX_create_index(thread_db* tdbb, gc_record->rec_flags &= ~REC_gc_active; if (primary.getWindow(tdbb).win_flags & WIN_large_scan) --relation->rel_scan_count; - ERR_post(Arg::Gds(isc_key_too_big)); + + context.raise(tdbb, idx_e_keytoobig, record); } UCHAR* p; @@ -468,10 +473,8 @@ void IDX_create_index(thread_db* tdbb, if (record != gc_record) delete record; } while (stack.hasData() && (record = stack.pop())); - gc_record->rec_flags &= ~REC_gc_active; - if (primary.getWindow(tdbb).win_flags & WIN_large_scan) - --relation->rel_scan_count; - ERR_post(Arg::Gds(isc_no_dup) << Arg::Str(index_name)); + + break; } USHORT l = key.key_length; @@ -497,6 +500,9 @@ void IDX_create_index(thread_db* tdbb, delete record; } + if (ifl_data.ifl_duplicates > 0) + break; + if (--tdbb->tdbb_quantum < 0) JRD_reschedule(tdbb, 0, true); } @@ -505,10 +511,29 @@ void IDX_create_index(thread_db* tdbb, if (primary.getWindow(tdbb).win_flags & WIN_large_scan) --relation->rel_scan_count; - SORT_sort(tdbb, sort_handle); + if (!ifl_data.ifl_duplicates) + SORT_sort(tdbb, sort_handle); - if (ifl_data.ifl_duplicates > 0) { - ERR_post(Arg::Gds(isc_no_dup) << Arg::Str(index_name)); + if (ifl_data.ifl_duplicates > 0) + { + AutoPtr error_record; + primary.rpb_record = NULL; + fb_assert(ifl_data.ifl_dup_recno >= 0); + primary.rpb_number.setValue(ifl_data.ifl_dup_recno); + + if (DPM_get(tdbb, &primary, LCK_read)) + { + if (primary.rpb_flags & rpb_deleted) + CCH_RELEASE(tdbb, &primary.getWindow(tdbb)); + else + { + VIO_data(tdbb, &primary, dbb->dbb_permanent); + error_record = primary.rpb_record; + } + + } + + context.raise(tdbb, idx_e_duplicate, error_record); } } @@ -521,12 +546,6 @@ void IDX_create_index(thread_db* tdbb, BTR_create(tdbb, relation, idx, key_length, sort_handle, selectivity); - if (ifl_data.ifl_duplicates > 0) - { - // we don't need SORT_fini() here, as it's called inside BTR_create() - ERR_post(Arg::Gds(isc_no_dup) << Arg::Str(index_name)); - } - if ((relation->rel_flags & REL_temp_conn) && (relation->getPages(tdbb)->rel_instance_id != 0)) { @@ -664,8 +683,7 @@ void IDX_delete_indices(thread_db* tdbb, jrd_rel* relation, RelationPages* relPa } -idx_e IDX_erase(thread_db* tdbb, record_param* rpb, - jrd_tra* transaction, jrd_rel** bad_relation, USHORT* bad_index) +void IDX_erase(thread_db* tdbb, record_param* rpb, jrd_tra* transaction) { /************************************** * @@ -679,12 +697,12 @@ idx_e IDX_erase(thread_db* tdbb, record_param* rpb, * a duplicate record. * **************************************/ - index_desc idx; SET_TDBB(tdbb); - idx_e error_code = idx_e_ok; + index_desc idx; idx.idx_id = idx_invalid; + RelationPages* relPages = rpb->rpb_relation->getPages(tdbb); WIN window(relPages->rel_pg_space_id, -1); @@ -692,17 +710,17 @@ idx_e IDX_erase(thread_db* tdbb, record_param* rpb, { if (idx.idx_flags & (idx_primary | idx_unique)) { - error_code = check_foreign_key(tdbb, rpb->rpb_record, rpb->rpb_relation, transaction, - &idx, bad_relation, bad_index); + IndexErrorContext context(rpb->rpb_relation, &idx); + + const idx_e error_code = check_foreign_key(tdbb, rpb->rpb_record, rpb->rpb_relation, transaction, + &idx, context); if (idx_e_ok != error_code) { CCH_RELEASE(tdbb, &window); - break; + context.raise(tdbb, error_code, rpb->rpb_record); } } } - - return error_code; } @@ -742,6 +760,8 @@ void IDX_garbage_collect(thread_db* tdbb, { if (BTR_description(tdbb, rpb->rpb_relation, root, &idx, i)) { + IndexErrorContext context(rpb->rpb_relation, &idx); + for (RecordStack::iterator stack1(going); stack1.hasData(); ++stack1) { Record* rec1 = stack1.object(); @@ -750,7 +770,7 @@ void IDX_garbage_collect(thread_db* tdbb, if (result != idx_e_ok) { CCH_RELEASE(tdbb, &window); - ERR_duplicate_error(result, rpb->rpb_relation, idx.idx_id); + context.raise(tdbb, result, rec1); } // Cancel index if there are duplicates in the remaining records @@ -764,7 +784,7 @@ void IDX_garbage_collect(thread_db* tdbb, if (result != idx_e_ok) { CCH_RELEASE(tdbb, &window); - ERR_duplicate_error(result, rpb->rpb_relation, idx.idx_id); + context.raise(tdbb, result, rec2); } if (key_equal(&key1, &key2)) @@ -784,7 +804,7 @@ void IDX_garbage_collect(thread_db* tdbb, if (result != idx_e_ok) { CCH_RELEASE(tdbb, &window); - ERR_duplicate_error(result, rpb->rpb_relation, idx.idx_id); + context.raise(tdbb, result, rec3); } if (key_equal(&key1, &key2)) @@ -809,10 +829,10 @@ void IDX_garbage_collect(thread_db* tdbb, } -idx_e IDX_modify(thread_db* tdbb, - record_param* org_rpb, - record_param* new_rpb, - jrd_tra* transaction, jrd_rel** bad_relation, USHORT* bad_index) +void IDX_modify(thread_db* tdbb, + record_param* org_rpb, + record_param* new_rpb, + jrd_tra* transaction) { /************************************** * @@ -828,8 +848,10 @@ idx_e IDX_modify(thread_db* tdbb, **************************************/ SET_TDBB(tdbb); - temporary_key key1; + temporary_key key1, key2; + index_desc idx; + idx.idx_id = idx_invalid; index_insertion insertion; insertion.iib_relation = org_rpb->rpb_relation; @@ -837,46 +859,45 @@ idx_e IDX_modify(thread_db* tdbb, insertion.iib_key = &key1; insertion.iib_descriptor = &idx; insertion.iib_transaction = transaction; - idx_e error_code = idx_e_ok; - idx.idx_id = idx_invalid; RelationPages* relPages = org_rpb->rpb_relation->getPages(tdbb); WIN window(relPages->rel_pg_space_id, -1); - temporary_key key2; - while (BTR_next_index(tdbb, org_rpb->rpb_relation, transaction, &idx, &window)) { - *bad_index = idx.idx_id; - *bad_relation = new_rpb->rpb_relation; - if ((error_code = - BTR_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, &idx, &key1, 0, false)) || - (error_code = - BTR_key(tdbb, org_rpb->rpb_relation, org_rpb->rpb_record, &idx, &key2, 0, false))) + IndexErrorContext context(new_rpb->rpb_relation, &idx); + idx_e error_code; + + if ( (error_code = + BTR_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, &idx, &key1, 0, false)) ) { CCH_RELEASE(tdbb, &window); - break; + context.raise(tdbb, error_code, new_rpb->rpb_record); } + + if ( (error_code = + BTR_key(tdbb, org_rpb->rpb_relation, org_rpb->rpb_record, &idx, &key2, 0, false)) ) + { + CCH_RELEASE(tdbb, &window); + context.raise(tdbb, error_code, org_rpb->rpb_record); + } + if (!key_equal(&key1, &key2)) { - if (( error_code = insert_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, - transaction, &window, &insertion, bad_relation, - bad_index)) ) + if ( (error_code = insert_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, + transaction, &window, &insertion, context)) ) { - return error_code; + context.raise(tdbb, error_code, new_rpb->rpb_record); } } } - - return error_code; } -idx_e IDX_modify_check_constraints(thread_db* tdbb, - record_param* org_rpb, - record_param* new_rpb, - jrd_tra* transaction, - jrd_rel** bad_relation, USHORT* bad_index) +void IDX_modify_check_constraints(thread_db* tdbb, + record_param* org_rpb, + record_param* new_rpb, + jrd_tra* transaction) { /************************************** * @@ -888,11 +909,10 @@ idx_e IDX_modify_check_constraints(thread_db* tdbb, * Check for foreign key constraint after a modify statement * **************************************/ - temporary_key key1, key2; - SET_TDBB(tdbb); - idx_e error_code = idx_e_ok; + temporary_key key1, key2; + index_desc idx; idx.idx_id = idx_invalid; @@ -905,7 +925,7 @@ idx_e IDX_modify_check_constraints(thread_db* tdbb, if (!(org_rpb->rpb_relation->rel_flags & REL_check_partners) && !(org_rpb->rpb_relation->rel_primary_dpnds.prim_reference_ids)) { - return error_code; + return; } // Now check all the foreign key constraints. Referential integrity relation @@ -916,31 +936,36 @@ idx_e IDX_modify_check_constraints(thread_db* tdbb, if (!(idx.idx_flags & (idx_primary | idx_unique)) || !MET_lookup_partner(tdbb, org_rpb->rpb_relation, &idx, 0)) { - continue; + continue; } - *bad_index = idx.idx_id; - *bad_relation = new_rpb->rpb_relation; - if ((error_code = - BTR_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, &idx, &key1, 0, false)) || - (error_code = - BTR_key(tdbb, org_rpb->rpb_relation, org_rpb->rpb_record, &idx, &key2, 0, false))) + + IndexErrorContext context(new_rpb->rpb_relation, &idx); + idx_e error_code; + + if ( (error_code = + BTR_key(tdbb, new_rpb->rpb_relation, new_rpb->rpb_record, &idx, &key1, 0, false)) ) { CCH_RELEASE(tdbb, &window); - break; + context.raise(tdbb, error_code, new_rpb->rpb_record); } + + if ( (error_code = + BTR_key(tdbb, org_rpb->rpb_relation, org_rpb->rpb_record, &idx, &key2, 0, false)) ) + { + CCH_RELEASE(tdbb, &window); + context.raise(tdbb, error_code, org_rpb->rpb_record); + } + if (!key_equal(&key1, &key2)) { - error_code = check_foreign_key(tdbb, org_rpb->rpb_record, org_rpb->rpb_relation, - transaction, &idx, bad_relation, bad_index); - if (idx_e_ok != error_code) + if ( (error_code = check_foreign_key(tdbb, org_rpb->rpb_record, org_rpb->rpb_relation, + transaction, &idx, context)) ) { CCH_RELEASE(tdbb, &window); - return error_code; + context.raise(tdbb, error_code, org_rpb->rpb_record); } } } - - return error_code; } @@ -964,8 +989,7 @@ void IDX_statistics(thread_db* tdbb, jrd_rel* relation, USHORT id, SelectivityLi } -idx_e IDX_store(thread_db* tdbb, record_param* rpb, - jrd_tra* transaction, jrd_rel** bad_relation, USHORT* bad_index) +void IDX_store(thread_db* tdbb, record_param* rpb, jrd_tra* transaction) { /************************************** * @@ -982,7 +1006,9 @@ idx_e IDX_store(thread_db* tdbb, record_param* rpb, SET_TDBB(tdbb); temporary_key key; + index_desc idx; + idx.idx_id = idx_invalid; index_insertion insertion; insertion.iib_relation = rpb->rpb_relation; @@ -991,29 +1017,26 @@ idx_e IDX_store(thread_db* tdbb, record_param* rpb, insertion.iib_descriptor = &idx; insertion.iib_transaction = transaction; - idx_e error_code = idx_e_ok; - idx.idx_id = idx_invalid; - RelationPages* relPages = rpb->rpb_relation->getPages(tdbb); WIN window(relPages->rel_pg_space_id, -1); while (BTR_next_index(tdbb, rpb->rpb_relation, transaction, &idx, &window)) { - *bad_index = idx.idx_id; - *bad_relation = rpb->rpb_relation; + IndexErrorContext context(rpb->rpb_relation, &idx); + idx_e error_code; + if ( (error_code = BTR_key(tdbb, rpb->rpb_relation, rpb->rpb_record, &idx, &key, 0, false)) ) { CCH_RELEASE(tdbb, &window); - break; + context.raise(tdbb, error_code, rpb->rpb_record); } + if ( (error_code = insert_key(tdbb, rpb->rpb_relation, rpb->rpb_record, transaction, - &window, &insertion, bad_relation, bad_index)) ) + &window, &insertion, context)) ) { - return error_code; + context.raise(tdbb, error_code, rpb->rpb_record); } } - - return error_code; } @@ -1236,8 +1259,7 @@ static idx_e check_foreign_key(thread_db* tdbb, jrd_rel* relation, jrd_tra* transaction, index_desc* idx, - jrd_rel** bad_relation, - USHORT* bad_index) + IndexErrorContext& context) { /************************************** * @@ -1316,15 +1338,9 @@ static idx_e check_foreign_key(thread_db* tdbb, if (result) { if (idx->idx_flags & idx_foreign) - { - *bad_relation = relation; - *bad_index = idx->idx_id; - } + context.setErrorLocation(relation, idx->idx_id); else - { - *bad_relation = partner_relation; - *bad_index = index_id; - } + context.setErrorLocation(partner_relation, index_id); } return result; @@ -1481,7 +1497,8 @@ static bool duplicate_key(const UCHAR* record1, const UCHAR* record2, void* ifl_ if (!(rec1->isr_flags & (ISR_secondary | ISR_null)) && !(rec2->isr_flags & (ISR_secondary | ISR_null))) { - ++ifl_data->ifl_duplicates; + if (!ifl_data->ifl_duplicates++) + ifl_data->ifl_dup_recno = rec2->isr_record_number; } return false; @@ -1552,8 +1569,7 @@ static idx_e insert_key(thread_db* tdbb, jrd_tra* transaction, WIN * window_ptr, index_insertion* insertion, - jrd_rel** bad_relation, - USHORT* bad_index) + IndexErrorContext& context) { /************************************** * @@ -1606,7 +1622,7 @@ static idx_e insert_key(thread_db* tdbb, if (result == idx_e_ok && null_state == idx_nulls_none) { result = check_foreign_key(tdbb, record, insertion->iib_relation, - transaction, idx, bad_relation, bad_index); + transaction, idx, context); } } diff --git a/src/jrd/idx_proto.h b/src/jrd/idx_proto.h index 8ae99be6a7..2d8441d463 100644 --- a/src/jrd/idx_proto.h +++ b/src/jrd/idx_proto.h @@ -33,11 +33,8 @@ namespace Jrd { class jrd_tra; struct record_param; class IndexBlock; - enum idx_e; struct index_desc; class CompilerScratch; - class jrd_fld; - class Record; } void IDX_check_access(Jrd::thread_db*, Jrd::CompilerScratch*, Jrd::jrd_rel*, Jrd::jrd_rel*); @@ -47,14 +44,12 @@ void IDX_create_index(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::index_desc*, const TE Jrd::IndexBlock* IDX_create_index_block(Jrd::thread_db*, Jrd::jrd_rel*, USHORT); void IDX_delete_index(Jrd::thread_db*, Jrd::jrd_rel*, USHORT); void IDX_delete_indices(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::RelationPages*); -Jrd::idx_e IDX_erase(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*, Jrd::jrd_rel**, USHORT*); +void IDX_erase(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*); void IDX_garbage_collect(Jrd::thread_db*, Jrd::record_param*, Jrd::RecordStack&, Jrd::RecordStack&); -Jrd::idx_e IDX_modify(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, - Jrd::jrd_tra*, Jrd::jrd_rel**, USHORT *); -Jrd::idx_e IDX_modify_check_constraints(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, - Jrd::jrd_tra*, Jrd::jrd_rel**, USHORT*); +void IDX_modify(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, Jrd::jrd_tra*); +void IDX_modify_check_constraints(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, Jrd::jrd_tra*); void IDX_statistics(Jrd::thread_db*, Jrd::jrd_rel*, USHORT, Jrd::SelectivityList&); -Jrd::idx_e IDX_store(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*, Jrd::jrd_rel**, USHORT*); +void IDX_store(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*); void IDX_modify_flag_uk_modified(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, Jrd::jrd_tra*); diff --git a/src/jrd/nav.cpp b/src/jrd/nav.cpp index 83b9082678..5f8c87d755 100644 --- a/src/jrd/nav.cpp +++ b/src/jrd/nav.cpp @@ -912,7 +912,10 @@ static bool get_record(thread_db* tdbb, RecordSource* rsb, IRSB_NAV impure, &value, 0, false); if (result != idx_e_ok) - ERR_duplicate_error(result, rpb->rpb_relation, idx->idx_id); + { + IndexErrorContext context(rpb->rpb_relation, idx); + context.raise(tdbb, result, rpb->rpb_record); + } if (compare_keys(idx, key->key_data, key->key_length, &value, 0)) return false;