From a91536203034816d15d850f0cba6b3c0c88a2e87 Mon Sep 17 00:00:00 2001 From: AlexPeshkoff Date: Tue, 5 Mar 2024 19:54:52 +0300 Subject: [PATCH] Postfix for #7997: Unexpected results when comparing integer with string containing value out of range of that integer datatype; fixed overflows that happen when index key is composed (cherry picked from commit 337ca497a09cb365937a3f00364aba445eb1d09b) --- src/jrd/btr.cpp | 69 +++++++++++++++++++++++-------- src/jrd/btr.h | 4 ++ src/jrd/btr_proto.h | 4 +- src/jrd/recsrc/IndexTableScan.cpp | 4 +- 4 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/jrd/btr.cpp b/src/jrd/btr.cpp index 835d8b87abf..db2d8cb0183 100644 --- a/src/jrd/btr.cpp +++ b/src/jrd/btr.cpp @@ -188,7 +188,8 @@ namespace static ULONG add_node(thread_db*, WIN*, index_insertion*, temporary_key*, RecordNumber*, ULONG*, ULONG*); -static void compress(thread_db*, const dsc*, const SSHORT, temporary_key*, USHORT, bool, bool, USHORT); +static void compress(thread_db*, const dsc*, const SSHORT, temporary_key*, USHORT, bool, bool, + USHORT, bool*); static USHORT compress_root(thread_db*, index_root_page*); static void copy_key(const temporary_key*, temporary_key*); static contents delete_node(thread_db*, WIN*, UCHAR*); @@ -222,7 +223,7 @@ 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*, const IndexRetrieval*, USHORT, temporary_key*, - bool&, const temporary_key&); + bool&, const temporary_key&, USHORT); static void update_selectivity(index_root_page*, USHORT, const SelectivityList&); static void checkForLowerKeySkip(bool&, const bool, const IndexNode&, const temporary_key&, const index_desc&, const IndexRetrieval*); @@ -721,14 +722,15 @@ void BTR_evaluate(thread_db* tdbb, const IndexRetrieval* retrieval, RecordBitmap temporary_key* lower = &lowerKey; temporary_key* upper = &upperKey; bool first = true; + USHORT forceInclFlag = 0; do { - btree_page* page = BTR_find_page(tdbb, retrieval, &window, &idx, lower, upper, first); + btree_page* page = BTR_find_page(tdbb, retrieval, &window, &idx, lower, upper, forceInclFlag, first); first = false; const bool descending = (idx.idx_flags & idx_descending); - bool skipLowerKey = (retrieval->irb_generic & irb_exclude_lower); + bool skipLowerKey = (retrieval->irb_generic & ~forceInclFlag) & irb_exclude_lower; const bool partLower = (retrieval->irb_lower_count < idx.idx_count); // If there is a starting descriptor, search down index to starting position. @@ -769,7 +771,7 @@ void BTR_evaluate(thread_db* tdbb, const IndexRetrieval* retrieval, RecordBitmap if (retrieval->irb_upper_count) { while (scan(tdbb, pointer, bitmap, bitmap_and, &idx, retrieval, prefix, upper, - skipLowerKey, *lower)) + skipLowerKey, *lower, forceInclFlag)) { page = (btree_page*) CCH_HANDOFF(tdbb, &window, page->btr_sibling, LCK_read, pag_index); pointer = page->btr_nodes + page->btr_jump_size; @@ -864,6 +866,7 @@ btree_page* BTR_find_page(thread_db* tdbb, index_desc* idx, temporary_key* lower, temporary_key* upper, + USHORT& forceInclFlag, bool makeKeys) { /************************************** @@ -898,21 +901,29 @@ btree_page* BTR_find_page(thread_db* tdbb, (retrieval->irb_desc.idx_flags & idx_unique) ? INTL_KEY_UNIQUE : INTL_KEY_SORT; + forceInclFlag &= ~(irb_force_lower | irb_force_upper); + if (retrieval->irb_upper_count) { + bool forceInclude = false; errorCode = BTR_make_key(tdbb, retrieval->irb_upper_count, retrieval->irb_value + retrieval->irb_desc.idx_count, retrieval->irb_scale, &retrieval->irb_desc, upper, - keyType); + keyType, &forceInclude); + if (forceInclude) + forceInclFlag |= irb_force_upper; } if (errorCode == idx_e_ok) { if (retrieval->irb_lower_count) { + bool forceInclude = false; errorCode = BTR_make_key(tdbb, retrieval->irb_lower_count, retrieval->irb_value, retrieval->irb_scale, &retrieval->irb_desc, lower, - keyType); + keyType, &forceInclude); + if (forceInclude) + forceInclFlag |= irb_force_lower; } } @@ -1255,7 +1266,7 @@ idx_e BTR_key(thread_db* tdbb, jrd_rel* relation, Record* record, index_desc* id key->key_flags |= key_empty; - compress(tdbb, desc_ptr, 0, key, tail->idx_itype, isNull, descending, keyType); + compress(tdbb, desc_ptr, 0, key, tail->idx_itype, isNull, descending, keyType, nullptr); } else { @@ -1290,7 +1301,7 @@ idx_e BTR_key(thread_db* tdbb, jrd_rel* relation, Record* record, index_desc* id } } - compress(tdbb, desc_ptr, 0, &temp, tail->idx_itype, isNull, descending, keyType); + compress(tdbb, desc_ptr, 0, &temp, tail->idx_itype, isNull, descending, keyType, nullptr); const UCHAR* q = temp.key_data; for (USHORT l = temp.key_length; l; --l, --stuff_count) @@ -1523,7 +1534,8 @@ idx_e BTR_make_key(thread_db* tdbb, const SSHORT* scale, const index_desc* idx, temporary_key* key, - USHORT keyType) + USHORT keyType, + bool* forceInclude) { /************************************** * @@ -1569,7 +1581,7 @@ idx_e BTR_make_key(thread_db* tdbb, if (isNull) key->key_nulls = 1; - compress(tdbb, desc, scale ? *scale : 0, key, tail->idx_itype, isNull, descending, keyType); + compress(tdbb, desc, scale ? *scale : 0, key, tail->idx_itype, isNull, descending, keyType, forceInclude); if (fuzzy && (key->key_flags & key_empty)) { @@ -1605,7 +1617,8 @@ idx_e BTR_make_key(thread_db* tdbb, compress(tdbb, desc, scale ? *scale++ : 0, &temp, tail->idx_itype, isNull, descending, (n == count - 1 ? - keyType : ((idx->idx_flags & idx_unique) ? INTL_KEY_UNIQUE : INTL_KEY_SORT))); + keyType : ((idx->idx_flags & idx_unique) ? INTL_KEY_UNIQUE : INTL_KEY_SORT)), + forceInclude); if (!(temp.key_flags & key_empty)) is_key_empty = false; @@ -1737,7 +1750,7 @@ void BTR_make_null_key(thread_db* tdbb, const index_desc* idx, temporary_key* ke // If the index is a single segment index, don't sweat the compound stuff if ((idx->idx_count == 1) || (idx->idx_flags & idx_expressn)) { - compress(tdbb, &null_desc, 0, key, tail->idx_itype, true, descending, INTL_KEY_SORT); + compress(tdbb, &null_desc, 0, key, tail->idx_itype, true, descending, INTL_KEY_SORT, nullptr); } else { @@ -1751,7 +1764,7 @@ void BTR_make_null_key(thread_db* tdbb, const index_desc* idx, temporary_key* ke for (; stuff_count; --stuff_count) *p++ = 0; - compress(tdbb, &null_desc, 0, &temp, tail->idx_itype, true, descending, INTL_KEY_SORT); + compress(tdbb, &null_desc, 0, &temp, tail->idx_itype, true, descending, INTL_KEY_SORT, nullptr); const UCHAR* q = temp.key_data; for (USHORT l = temp.key_length; l; --l, --stuff_count) @@ -2450,7 +2463,8 @@ static void compress(thread_db* tdbb, const SSHORT matchScale, temporary_key* key, USHORT itype, - bool isNull, bool descending, USHORT key_type) + bool isNull, bool descending, USHORT key_type, + bool* forceInclude) { /************************************** * @@ -2662,7 +2676,27 @@ static void compress(thread_db* tdbb, else if (itype == idx_numeric2) { int64_key_op = true; - temp.temp_int64_key = make_int64_key(MOV_get_int64(tdbb, desc, scale), scale); + SINT64 v = 0; + try + { + v = MOV_get_int64(tdbb, desc, scale); + } + catch (const Exception& ex) + { + ex.stuffException(tdbb->tdbb_status_vector); + const ISC_STATUS* st = tdbb->tdbb_status_vector->getErrors(); + if (!(fb_utils::containsErrorCode(st, isc_arith_except) || + fb_utils::containsErrorCode(st, isc_decfloat_invalid_operation))) + { + throw; + } + + tdbb->tdbb_status_vector->init(); + v = MOV_get_dec128(tdbb, desc).sign() < 0 ? MIN_SINT64 : MAX_SINT64; + if (forceInclude) + *forceInclude = true; + } + temp.temp_int64_key = make_int64_key(v, scale); temp_copy_length = sizeof(temp.temp_int64_key.d_part); temp_is_negative = (temp.temp_int64_key.d_part < 0); @@ -6405,7 +6439,7 @@ static contents remove_leaf_node(thread_db* tdbb, index_insertion* insertion, WI static bool scan(thread_db* tdbb, UCHAR* pointer, RecordBitmap** bitmap, RecordBitmap* bitmap_and, index_desc* idx, const IndexRetrieval* retrieval, USHORT prefix, temporary_key* key, - bool& skipLowerKey, const temporary_key& lowerKey) + bool& skipLowerKey, const temporary_key& lowerKey, USHORT forceInclFlag) { /************************************** * @@ -6428,6 +6462,7 @@ static bool scan(thread_db* tdbb, UCHAR* pointer, RecordBitmap** bitmap, RecordB // stuff the key to the stuff boundary ULONG count; USHORT flag = retrieval->irb_generic; + flag &= ~forceInclFlag; // clear exclude bits if needed if ((flag & irb_partial) && (flag & irb_equality) && !(flag & irb_starting) && !(flag & irb_descending)) diff --git a/src/jrd/btr.h b/src/jrd/btr.h index b2fbce9ef8c..40e93acd4d8 100644 --- a/src/jrd/btr.h +++ b/src/jrd/btr.h @@ -233,6 +233,10 @@ const int irb_exclude_lower = 32; // exclude lower bound keys while scanning i const int irb_exclude_upper = 64; // exclude upper bound keys while scanning index const int irb_multi_starting = 128; // Use INTL_KEY_MULTI_STARTING +// Force include flags - always include appropriate key while scanning index +const int irb_force_lower = irb_exclude_lower; +const int irb_force_upper = irb_exclude_upper; + typedef Firebird::HalfStaticArray SelectivityList; class BtrPageGCLock : public Lock diff --git a/src/jrd/btr_proto.h b/src/jrd/btr_proto.h index fde225cdbb5..1f04007f99c 100644 --- a/src/jrd/btr_proto.h +++ b/src/jrd/btr_proto.h @@ -38,7 +38,7 @@ DSC* BTR_eval_expression(Jrd::thread_db*, Jrd::index_desc*, Jrd::Record*, bool&) void BTR_evaluate(Jrd::thread_db*, const Jrd::IndexRetrieval*, Jrd::RecordBitmap**, Jrd::RecordBitmap*); UCHAR* BTR_find_leaf(Ods::btree_page*, Jrd::temporary_key*, UCHAR*, USHORT*, bool, int); Ods::btree_page* BTR_find_page(Jrd::thread_db*, const Jrd::IndexRetrieval*, Jrd::win*, Jrd::index_desc*, - Jrd::temporary_key*, Jrd::temporary_key*, bool = true); + Jrd::temporary_key*, Jrd::temporary_key*, USHORT&, bool = true); void BTR_insert(Jrd::thread_db*, Jrd::win*, Jrd::index_insertion*); Jrd::idx_e BTR_key(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::Record*, Jrd::index_desc*, Jrd::temporary_key*, const USHORT, USHORT = 0); @@ -46,7 +46,7 @@ USHORT BTR_key_length(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::index_desc*); Ods::btree_page* BTR_left_handoff(Jrd::thread_db*, Jrd::win*, Ods::btree_page*, SSHORT); bool BTR_lookup(Jrd::thread_db*, Jrd::jrd_rel*, USHORT, Jrd::index_desc*, Jrd::RelationPages*); Jrd::idx_e BTR_make_key(Jrd::thread_db*, USHORT, const Jrd::ValueExprNode* const*, const SSHORT* scale, - const Jrd::index_desc*, Jrd::temporary_key*, USHORT); + const Jrd::index_desc*, Jrd::temporary_key*, USHORT, bool*); void BTR_make_null_key(Jrd::thread_db*, const Jrd::index_desc*, Jrd::temporary_key*); bool BTR_next_index(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::jrd_tra*, Jrd::index_desc*, Jrd::win*); void BTR_remove(Jrd::thread_db*, Jrd::win*, Jrd::index_insertion*); diff --git a/src/jrd/recsrc/IndexTableScan.cpp b/src/jrd/recsrc/IndexTableScan.cpp index 9c3dd37d5c5..ab4ee8c7c8a 100644 --- a/src/jrd/recsrc/IndexTableScan.cpp +++ b/src/jrd/recsrc/IndexTableScan.cpp @@ -571,10 +571,12 @@ UCHAR* IndexTableScan::openStream(thread_db* tdbb, Impure* impure, win* window) setPage(tdbb, impure, NULL); impure->irsb_nav_length = 0; + USHORT dummy = 0; // exclude upper/lower bits are not used here, + // i.e. additional forced include not needed // Find the starting leaf page const IndexRetrieval* const retrieval = m_index->retrieval; index_desc* const idx = (index_desc*) ((SCHAR*) impure + m_offset); - Ods::btree_page* page = BTR_find_page(tdbb, retrieval, window, idx, lower, upper, firstKeys); + Ods::btree_page* page = BTR_find_page(tdbb, retrieval, window, idx, lower, upper, dummy, firstKeys); setPage(tdbb, impure, window); // find the upper limit for the search