From a98863af8c45d3bf9696cb4420f546cfe17aeadb Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Tue, 25 Nov 2025 17:22:22 +0300 Subject: [PATCH 1/6] Fixes a loop in the GENERATE_SERIES function on boundary values. --- src/jrd/recsrc/TableValueFunctionScan.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index 637d5f49cb6..53c70399d2b 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -522,7 +522,13 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt64(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - result += step; + // Fixes freezing at boundary values. + if (((step < 0) && (result == MIN_SINT64)) || + ((step > 0) && (result == MAX_SINT64))) + impure->m_step.vlu_int64 = 0; + else + result += step; + impure->m_result.vlu_int64 = result; return true; @@ -545,7 +551,12 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt128(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - result = result.add(step); + // Fixes freezing at boundary values. + if (((step.sign() < 0) && (result.compare(MIN_Int128) == 0)) || + ((step.sign() > 0) && (result.compare(MAX_Int128) == 0))) + impure->m_step.vlu_int128.set(0, 0); + else + result = result.add(step); impure->m_result.vlu_int128 = result; return true; From 9c552d0f23e51f4c1ca6a54189fd712fb8f06af7 Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Tue, 25 Nov 2025 17:47:43 +0300 Subject: [PATCH 2/6] Correction according to dyemanov --- src/jrd/recsrc/TableValueFunctionScan.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index 53c70399d2b..db95245b64a 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -522,10 +522,12 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt64(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - // Fixes freezing at boundary values. - if (((step < 0) && (result == MIN_SINT64)) || + // Prevent freezing at boundary values + if (((step < 0) && (result == MIN_SINT64)) || ((step > 0) && (result == MAX_SINT64))) + { impure->m_step.vlu_int64 = 0; + } else result += step; @@ -551,10 +553,12 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt128(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - // Fixes freezing at boundary values. + // Prevent freezing at boundary values if (((step.sign() < 0) && (result.compare(MIN_Int128) == 0)) || ((step.sign() > 0) && (result.compare(MAX_Int128) == 0))) + { impure->m_step.vlu_int128.set(0, 0); + } else result = result.add(step); impure->m_result.vlu_int128 = result; From 96f091d958ee2abfde0c5df07e0527dabcbe9497 Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Wed, 26 Nov 2025 11:23:57 +0300 Subject: [PATCH 3/6] A more complete solution for taking into account boundary values --- src/jrd/recsrc/TableValueFunctionScan.cpp | 43 ++++++++++++++++++----- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index db95245b64a..39f5e2c6329 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -522,12 +522,25 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt64(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - // Prevent freezing at boundary values - if (((step < 0) && (result == MIN_SINT64)) || - ((step > 0) && (result == MAX_SINT64))) + // Prevents overflows for SINT64 type at boundary values + bool reachedBoundary = false; + if (step > 0) { - impure->m_step.vlu_int64 = 0; + if (result > MAX_SINT64 - step) + reachedBoundary = true; + else + reachedBoundary = (result + step > finish); + } + else if (step < 0) + { + if (result < MIN_SINT64 - step) + reachedBoundary = true; + else + reachedBoundary = (result + step < finish); } + + if ((result == finish) || reachedBoundary) + impure->m_step.vlu_int64 = 0; else result += step; @@ -553,14 +566,28 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const fromDesc.makeInt128(impure->m_scale, &result); assignParameter(tdbb, &fromDesc, toDesc, 0, record); - // Prevent freezing at boundary values - if (((step.sign() < 0) && (result.compare(MIN_Int128) == 0)) || - ((step.sign() > 0) && (result.compare(MAX_Int128) == 0))) + // Prevents overflows for INT128 type at boundary values + bool reachedBoundary = false; + if (step.sign() > 0) { - impure->m_step.vlu_int128.set(0, 0); + if (result.compare(MAX_Int128.sub(step)) > 0) + reachedBoundary = true; + else + reachedBoundary = (result.add(step).compare(finish) > 0); } + else if (step.sign() < 0) + { + if (result.compare(MIN_Int128.sub(step)) < 0) + reachedBoundary = true; + else + reachedBoundary = (result.add(step).compare(finish) < 0); + } + + if (reachedBoundary || (result.compare(finish) == 0)) + impure->m_step.vlu_int128.set(0, 0); else result = result.add(step); + impure->m_result.vlu_int128 = result; return true; From 7996aef4a721815065cdfc626a6b404013a19c2f Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Fri, 28 Nov 2025 16:56:33 +0300 Subject: [PATCH 4/6] Refactoring calculation with different types. ArithmeticNode::add is now used as agreed upon by @asfernandes. --- src/jrd/recsrc/RecordSource.h | 30 +--- src/jrd/recsrc/TableValueFunctionScan.cpp | 178 +++++++--------------- 2 files changed, 62 insertions(+), 146 deletions(-) diff --git a/src/jrd/recsrc/RecordSource.h b/src/jrd/recsrc/RecordSource.h index 2b81225ec63..236c256c790 100644 --- a/src/jrd/recsrc/RecordSource.h +++ b/src/jrd/recsrc/RecordSource.h @@ -1626,32 +1626,14 @@ namespace Jrd struct Impure final : public TableValueFunctionScan::Impure { - union - { - SINT64 vlu_int64; - Firebird::Int128 vlu_int128; - } m_start; - - union - { - SINT64 vlu_int64; - Firebird::Int128 vlu_int128; - } m_finish; - - union - { - SINT64 vlu_int64; - Firebird::Int128 vlu_int128; - } m_step; - - union - { - SINT64 vlu_int64; - Firebird::Int128 vlu_int128; - } m_result; + impure_value m_start; + impure_value m_finish; + impure_value m_step; + impure_value m_result; - UCHAR m_dtype; + USHORT m_flags; SCHAR m_scale; + SCHAR m_stepSign; }; public: diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index 39f5e2c6329..6e16539393b 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -407,54 +407,41 @@ void GenSeriesFunctionScan::internalOpen(thread_db* tdbb) const const auto impure = request->getImpure(m_impure); impure->m_recordBuffer = nullptr; - // common scale - impure->m_scale = MIN(MIN(startDesc->dsc_scale, finishDesc->dsc_scale), stepDesc->dsc_scale); - // common type - impure->m_dtype = MAX(MAX(startDesc->dsc_dtype, finishDesc->dsc_dtype), stepDesc->dsc_dtype); + ArithmeticNode::getDescDialect3(tdbb, &impure->m_result.vlu_desc, *startDesc, *stepDesc, blr_add, &impure->m_scale, &impure->m_flags); - if (impure->m_dtype != dtype_int128) - { - const auto start = MOV_get_int64(tdbb, startDesc, impure->m_scale); - const auto finish = MOV_get_int64(tdbb, finishDesc, impure->m_scale); - const auto step = MOV_get_int64(tdbb, stepDesc, impure->m_scale); - - if (step == 0) - status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name)); + SLONG zero = 0; + dsc zeroDesc; + zeroDesc.makeLong(0, &zero); + impure->m_stepSign = MOV_compare(tdbb, stepDesc, &zeroDesc); - // validate parameter value - if (((step > 0) && (start > finish)) || - ((step < 0) && (start < finish))) - { - return; - } + if (impure->m_stepSign == 0) + status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name)); - impure->m_start.vlu_int64 = start; - impure->m_finish.vlu_int64 = finish; - impure->m_step.vlu_int64 = step; - impure->m_result.vlu_int64 = start; - } - else + const auto boundaryComparison = MOV_compare(tdbb, startDesc, finishDesc); + // validate parameter value + if (((impure->m_stepSign > 0) && (boundaryComparison > 0)) || + ((impure->m_stepSign < 0) && (boundaryComparison < 0))) { - const auto start = MOV_get_int128(tdbb, startDesc, impure->m_scale); - const auto finish = MOV_get_int128(tdbb, finishDesc, impure->m_scale); - const auto step = MOV_get_int128(tdbb, stepDesc, impure->m_scale); - - if (step.sign() == 0) - status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name)); - - // validate parameter value - if (((step.sign() > 0) && (start.compare(finish) > 0)) || - ((step.sign() < 0) && (start.compare(finish) < 0))) - { - return; - } - - impure->m_start.vlu_int128 = start; - impure->m_finish.vlu_int128 = finish; - impure->m_step.vlu_int128 = step; - impure->m_result.vlu_int128 = start; + return; } + EVL_make_value(tdbb, startDesc, &impure->m_start); + EVL_make_value(tdbb, finishDesc, &impure->m_finish); + EVL_make_value(tdbb, stepDesc, &impure->m_step); + + switch (impure->m_result.vlu_desc.dsc_dtype) { + case dtype_int64: + impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int64); + break; + case dtype_int128: + impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int128); + break; + default: + fb_assert(false); + } + // result = start + MOV_move(tdbb, startDesc, &impure->m_result.vlu_desc); + impure->irsb_flags |= irsb_open; VIO_record(tdbb, rpb, m_format, &pool); @@ -505,93 +492,40 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const const auto request = tdbb->getRequest(); const auto impure = request->getImpure(m_impure); - if (impure->m_dtype != dtype_int128) - { - auto result = impure->m_result.vlu_int64; - const auto finish = impure->m_finish.vlu_int64; - const auto step = impure->m_step.vlu_int64; - - if (((step > 0) && (result <= finish)) || - ((step < 0) && (result >= finish))) - { - Record* const record = request->req_rpb[m_stream].rpb_record; - - auto toDesc = m_format->fmt_desc.begin(); - dsc fromDesc; - fromDesc.makeInt64(impure->m_scale, &result); - assignParameter(tdbb, &fromDesc, toDesc, 0, record); - - // Prevents overflows for SINT64 type at boundary values - bool reachedBoundary = false; - if (step > 0) - { - if (result > MAX_SINT64 - step) - reachedBoundary = true; - else - reachedBoundary = (result + step > finish); - } - else if (step < 0) - { - if (result < MIN_SINT64 - step) - reachedBoundary = true; - else - reachedBoundary = (result + step < finish); - } + const auto ñomparison = MOV_compare(tdbb, &impure->m_result.vlu_desc, &impure->m_finish.vlu_desc); + if (((impure->m_stepSign > 0) && (ñomparison <= 0)) || + ((impure->m_stepSign < 0) && (ñomparison >= 0))) + { + Record* const record = request->req_rpb[m_stream].rpb_record; - if ((result == finish) || reachedBoundary) - impure->m_step.vlu_int64 = 0; - else - result += step; + auto toDesc = m_format->fmt_desc.begin(); - impure->m_result.vlu_int64 = result; + assignParameter(tdbb, &impure->m_result.vlu_desc, toDesc, 0, record); - return true; + // evaluate next result + try + { + impure_value nextValue; + + ArithmeticNode::add(tdbb, + &impure->m_step.vlu_desc, + &impure->m_result.vlu_desc, + &nextValue, + blr_add, + false, + impure->m_scale, + impure->m_flags); + + MOV_move(tdbb, &nextValue.vlu_desc, &impure->m_result.vlu_desc); } - } - else - { - auto result = impure->m_result.vlu_int128; - const auto finish = impure->m_finish.vlu_int128; - const auto step = impure->m_step.vlu_int128; - - if (((step.sign() > 0) && (result.compare(finish) <= 0)) || - ((step.sign() < 0) && (result.compare(finish) >= 0))) + catch (const status_exception&) { - Record* const record = request->req_rpb[m_stream].rpb_record; - - auto toDesc = m_format->fmt_desc.begin(); - - dsc fromDesc; - fromDesc.makeInt128(impure->m_scale, &result); - assignParameter(tdbb, &fromDesc, toDesc, 0, record); - - // Prevents overflows for INT128 type at boundary values - bool reachedBoundary = false; - if (step.sign() > 0) - { - if (result.compare(MAX_Int128.sub(step)) > 0) - reachedBoundary = true; - else - reachedBoundary = (result.add(step).compare(finish) > 0); - } - else if (step.sign() < 0) - { - if (result.compare(MIN_Int128.sub(step)) < 0) - reachedBoundary = true; - else - reachedBoundary = (result.add(step).compare(finish) < 0); - } - - if (reachedBoundary || (result.compare(finish) == 0)) - impure->m_step.vlu_int128.set(0, 0); - else - result = result.add(step); - - impure->m_result.vlu_int128 = result; - - return true; + tdbb->tdbb_status_vector->clearException(); + // stop evaluate next result + impure->m_stepSign = 0; } + return true; } return false; From f4936877c41b5c2a739b91d3f4f398bcea2b2e57 Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Fri, 28 Nov 2025 19:56:36 +0300 Subject: [PATCH 5/6] Fixed non-ASCII character in variable name --- src/jrd/recsrc/TableValueFunctionScan.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index 6e16539393b..dd94d0ae8c3 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -441,7 +441,7 @@ void GenSeriesFunctionScan::internalOpen(thread_db* tdbb) const } // result = start MOV_move(tdbb, startDesc, &impure->m_result.vlu_desc); - + impure->irsb_flags |= irsb_open; VIO_record(tdbb, rpb, m_format, &pool); @@ -493,9 +493,9 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const const auto impure = request->getImpure(m_impure); - const auto ñomparison = MOV_compare(tdbb, &impure->m_result.vlu_desc, &impure->m_finish.vlu_desc); - if (((impure->m_stepSign > 0) && (ñomparison <= 0)) || - ((impure->m_stepSign < 0) && (ñomparison >= 0))) + const auto comparison = MOV_compare(tdbb, &impure->m_result.vlu_desc, &impure->m_finish.vlu_desc); + if (((impure->m_stepSign > 0) && (comparison <= 0)) || + ((impure->m_stepSign < 0) && (comparison >= 0))) { Record* const record = request->req_rpb[m_stream].rpb_record; @@ -504,7 +504,7 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const assignParameter(tdbb, &impure->m_result.vlu_desc, toDesc, 0, record); // evaluate next result - try + try { impure_value nextValue; From 8ffc5addb097c136ff0560f007fd22207dc56376 Mon Sep 17 00:00:00 2001 From: Simonov Denis Date: Tue, 9 Dec 2025 14:51:29 +0300 Subject: [PATCH 6/6] Adjusting indents --- src/jrd/recsrc/TableValueFunctionScan.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/jrd/recsrc/TableValueFunctionScan.cpp b/src/jrd/recsrc/TableValueFunctionScan.cpp index dd94d0ae8c3..b0b5bbe30b3 100644 --- a/src/jrd/recsrc/TableValueFunctionScan.cpp +++ b/src/jrd/recsrc/TableValueFunctionScan.cpp @@ -429,16 +429,18 @@ void GenSeriesFunctionScan::internalOpen(thread_db* tdbb) const EVL_make_value(tdbb, finishDesc, &impure->m_finish); EVL_make_value(tdbb, stepDesc, &impure->m_step); - switch (impure->m_result.vlu_desc.dsc_dtype) { - case dtype_int64: - impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int64); - break; - case dtype_int128: - impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int128); - break; - default: - fb_assert(false); + switch (impure->m_result.vlu_desc.dsc_dtype) + { + case dtype_int64: + impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int64); + break; + case dtype_int128: + impure->m_result.vlu_desc.dsc_address = reinterpret_cast(&impure->m_result.vlu_misc.vlu_int128); + break; + default: + fb_assert(false); } + // result = start MOV_move(tdbb, startDesc, &impure->m_result.vlu_desc); @@ -492,7 +494,6 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const const auto request = tdbb->getRequest(); const auto impure = request->getImpure(m_impure); - const auto comparison = MOV_compare(tdbb, &impure->m_result.vlu_desc, &impure->m_finish.vlu_desc); if (((impure->m_stepSign > 0) && (comparison <= 0)) || ((impure->m_stepSign < 0) && (comparison >= 0))) @@ -525,6 +526,7 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const // stop evaluate next result impure->m_stepSign = 0; } + return true; }