Skip to content

Fixes a loop in the GENERATE_SERIES function on boundary values. #8812

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.

Sign up for GitHub

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

Open
sim1984 wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fixes a loop in the GENERATE_SERIES function on boundary values. #8812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a98863a
Fixes a loop in the GENERATE_SERIES function on boundary values.
sim1984 Nov 25, 2025
9c552d0
Correction according to dyemanov
sim1984 Nov 25, 2025
96f091d
A more complete solution for taking into account boundary values
sim1984 Nov 26, 2025
7996aef
Refactoring calculation with different types. ArithmeticNode::add is …
sim1984 Nov 28, 2025
f493687
Fixed non-ASCII character in variable name
sim1984 Nov 28, 2025
8ffc5ad
Adjusting indents
sim1984 Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 6 additions & 24 deletions src/jrd/recsrc/RecordSource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
134 changes: 56 additions & 78 deletions src/jrd/recsrc/TableValueFunctionScan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,54 +407,43 @@ void GenSeriesFunctionScan::internalOpen(thread_db* tdbb) const
const auto impure = request->getImpure<Impure>(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);
SLONG zero = 0;
dsc zeroDesc;
zeroDesc.makeLong(0, &zero);
impure->m_stepSign = MOV_compare(tdbb, stepDesc, &zeroDesc);

if (step == 0)
status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name));
if (impure->m_stepSign == 0)
status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name));

// validate parameter value
if (((step > 0) && (start > finish)) ||
((step < 0) && (start < finish)))
{
return;
}

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);
return;
}

if (step.sign() == 0)
status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name));
EVL_make_value(tdbb, startDesc, &impure->m_start);
EVL_make_value(tdbb, finishDesc, &impure->m_finish);
EVL_make_value(tdbb, stepDesc, &impure->m_step);

// 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;
switch (impure->m_result.vlu_desc.dsc_dtype)
{
case dtype_int64:
impure->m_result.vlu_desc.dsc_address = reinterpret_cast<UCHAR*>(&impure->m_result.vlu_misc.vlu_int64);
break;
case dtype_int128:
impure->m_result.vlu_desc.dsc_address = reinterpret_cast<UCHAR*>(&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);
Expand Down Expand Up @@ -505,51 +494,40 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const
const auto request = tdbb->getRequest();
const auto impure = request->getImpure<Impure>(m_impure);

if (impure->m_dtype != dtype_int128)
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)))
{
auto result = impure->m_result.vlu_int64;
const auto finish = impure->m_finish.vlu_int64;
const auto step = impure->m_step.vlu_int64;
Record* const record = request->req_rpb[m_stream].rpb_record;

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();

auto toDesc = m_format->fmt_desc.begin();

dsc fromDesc;
fromDesc.makeInt64(impure->m_scale, &result);
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
assignParameter(tdbb, &impure->m_result.vlu_desc, toDesc, 0, record);

result += step;
impure->m_result.vlu_int64 = result;

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);

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;
Copy link
Member

@asfernandes asfernandes Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return true;
return true;

Copy link
Contributor Author

@sim1984 sim1984 Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

return false;
Expand Down
Loading