Skip to content

Commit

Permalink
Postfix for #7997: Unexpected results when comparing integer with str…
Browse files Browse the repository at this point in the history 
…ing containing value out of range of that integer datatype; fixed overflows that happen when index key is composed

(cherry picked from commit 337ca49)
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed Mar 13, 2024
1 parent 2981fbd commit a915362
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 20 deletions.
69 changes: 52 additions & 17 deletions src/jrd/btr.cpp
View file
Expand Up @@ -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*);
Expand Down Expand Up @@ -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*);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
/**************************************
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
/**************************************
*
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand All @@ -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)
Expand Down Expand Up @@ -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)
{
/**************************************
*
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
{
/**************************************
*
Expand All @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions src/jrd/btr.h
View file
Expand Up @@ -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<float, 4> SelectivityList;

class BtrPageGCLock : public Lock
Expand Down
4 changes: 2 additions & 2 deletions src/jrd/btr_proto.h
View file
Expand Up @@ -38,15 +38,15 @@ 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);
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*);
Expand Down
4 changes: 3 additions & 1 deletion src/jrd/recsrc/IndexTableScan.cpp
View file
Expand Up @@ -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
Expand Down

0 comments on commit a915362

Please sign in to comment.