Skip to content

Commit ddc4402

Browse files
sim1984sim1984
authored
Fixes a loop in the GENERATE_SERIES function on boundary values. (#8812)
* Fixes a loop in the GENERATE_SERIES function on boundary values. * Correction according to dyemanov * A more complete solution for taking into account boundary values * Refactoring calculation with different types. ArithmeticNode::add is now used as agreed upon by @asfernandes. * Fixed non-ASCII character in variable name * Adjusting indents
1 parent 3b980c6 commit ddc4402

File tree

2 files changed

+62
-102
lines changed

2 files changed

+62
-102
lines changed

‎src/jrd/recsrc/RecordSource.h‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,32 +1626,14 @@ namespace Jrd
16261626

16271627
struct Impure final : public TableValueFunctionScan::Impure
16281628
{
1629-
union
1630-
{
1631-
SINT64 vlu_int64;
1632-
Firebird::Int128 vlu_int128;
1633-
} m_start;
1634-
1635-
union
1636-
{
1637-
SINT64 vlu_int64;
1638-
Firebird::Int128 vlu_int128;
1639-
} m_finish;
1640-
1641-
union
1642-
{
1643-
SINT64 vlu_int64;
1644-
Firebird::Int128 vlu_int128;
1645-
} m_step;
1646-
1647-
union
1648-
{
1649-
SINT64 vlu_int64;
1650-
Firebird::Int128 vlu_int128;
1651-
} m_result;
1629+
impure_value m_start;
1630+
impure_value m_finish;
1631+
impure_value m_step;
1632+
impure_value m_result;
16521633

1653-
UCHAR m_dtype;
1634+
USHORT m_flags;
16541635
SCHAR m_scale;
1636+
SCHAR m_stepSign;
16551637
};
16561638

16571639
public:

‎src/jrd/recsrc/TableValueFunctionScan.cpp‎

Lines changed: 56 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -407,54 +407,43 @@ void GenSeriesFunctionScan::internalOpen(thread_db* tdbb) const
407407
const auto impure = request->getImpure<Impure>(m_impure);
408408
impure->m_recordBuffer = nullptr;
409409

410-
// common scale
411-
impure->m_scale = MIN(MIN(startDesc->dsc_scale, finishDesc->dsc_scale), stepDesc->dsc_scale);
412-
// common type
413-
impure->m_dtype = MAX(MAX(startDesc->dsc_dtype, finishDesc->dsc_dtype), stepDesc->dsc_dtype);
410+
ArithmeticNode::getDescDialect3(tdbb, &impure->m_result.vlu_desc, *startDesc, *stepDesc, blr_add, &impure->m_scale, &impure->m_flags);
414411

415-
if (impure->m_dtype != dtype_int128)
416-
{
417-
const auto start = MOV_get_int64(tdbb, startDesc, impure->m_scale);
418-
const auto finish = MOV_get_int64(tdbb, finishDesc, impure->m_scale);
419-
const auto step = MOV_get_int64(tdbb, stepDesc, impure->m_scale);
412+
SLONG zero = 0;
413+
dsc zeroDesc;
414+
zeroDesc.makeLong(0, &zero);
415+
impure->m_stepSign = MOV_compare(tdbb, stepDesc, &zeroDesc);
420416

421-
if (step == 0)
422-
status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name));
417+
if (impure->m_stepSign == 0)
418+
status_exception::raise(Arg::Gds(isc_genseq_stepmustbe_nonzero) << Arg::Str(m_name));
423419

424-
// validate parameter value
425-
if (((step > 0) && (start > finish)) ||
426-
((step < 0) && (start < finish)))
427-
{
428-
return;
429-
}
430-
431-
impure->m_start.vlu_int64 = start;
432-
impure->m_finish.vlu_int64 = finish;
433-
impure->m_step.vlu_int64 = step;
434-
impure->m_result.vlu_int64 = start;
435-
}
436-
else
420+
const auto boundaryComparison = MOV_compare(tdbb, startDesc, finishDesc);
421+
// validate parameter value
422+
if (((impure->m_stepSign > 0) && (boundaryComparison > 0)) ||
423+
((impure->m_stepSign < 0) && (boundaryComparison < 0)))
437424
{
438-
const auto start = MOV_get_int128(tdbb, startDesc, impure->m_scale);
439-
const auto finish = MOV_get_int128(tdbb, finishDesc, impure->m_scale);
440-
const auto step = MOV_get_int128(tdbb, stepDesc, impure->m_scale);
425+
return;
426+
}
441427

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

445-
// validate parameter value
446-
if (((step.sign() > 0) && (start.compare(finish) > 0)) ||
447-
((step.sign() < 0) && (start.compare(finish) < 0)))
448-
{
449-
return;
450-
}
451-
452-
impure->m_start.vlu_int128 = start;
453-
impure->m_finish.vlu_int128 = finish;
454-
impure->m_step.vlu_int128 = step;
455-
impure->m_result.vlu_int128 = start;
432+
switch (impure->m_result.vlu_desc.dsc_dtype)
433+
{
434+
case dtype_int64:
435+
impure->m_result.vlu_desc.dsc_address = reinterpret_cast<UCHAR*>(&impure->m_result.vlu_misc.vlu_int64);
436+
break;
437+
case dtype_int128:
438+
impure->m_result.vlu_desc.dsc_address = reinterpret_cast<UCHAR*>(&impure->m_result.vlu_misc.vlu_int128);
439+
break;
440+
default:
441+
fb_assert(false);
456442
}
457443

444+
// result = start
445+
MOV_move(tdbb, startDesc, &impure->m_result.vlu_desc);
446+
458447
impure->irsb_flags |= irsb_open;
459448

460449
VIO_record(tdbb, rpb, m_format, &pool);
@@ -505,51 +494,40 @@ bool GenSeriesFunctionScan::nextBuffer(thread_db* tdbb) const
505494
const auto request = tdbb->getRequest();
506495
const auto impure = request->getImpure<Impure>(m_impure);
507496

508-
if (impure->m_dtype != dtype_int128)
497+
const auto comparison = MOV_compare(tdbb, &impure->m_result.vlu_desc, &impure->m_finish.vlu_desc);
498+
if (((impure->m_stepSign > 0) && (comparison <= 0)) ||
499+
((impure->m_stepSign < 0) && (comparison >= 0)))
509500
{
510-
auto result = impure->m_result.vlu_int64;
511-
const auto finish = impure->m_finish.vlu_int64;
512-
const auto step = impure->m_step.vlu_int64;
501+
Record* const record = request->req_rpb[m_stream].rpb_record;
513502

514-
if (((step > 0) && (result <= finish)) ||
515-
((step < 0) && (result >= finish)))
516-
{
517-
Record* const record = request->req_rpb[m_stream].rpb_record;
503+
auto toDesc = m_format->fmt_desc.begin();
518504

519-
auto toDesc = m_format->fmt_desc.begin();
520-
521-
dsc fromDesc;
522-
fromDesc.makeInt64(impure->m_scale, &result);
523-
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
505+
assignParameter(tdbb, &impure->m_result.vlu_desc, toDesc, 0, record);
524506

525-
result += step;
526-
impure->m_result.vlu_int64 = result;
527-
528-
return true;
507+
// evaluate next result
508+
try
509+
{
510+
impure_value nextValue;
511+
512+
ArithmeticNode::add(tdbb,
513+
&impure->m_step.vlu_desc,
514+
&impure->m_result.vlu_desc,
515+
&nextValue,
516+
blr_add,
517+
false,
518+
impure->m_scale,
519+
impure->m_flags);
520+
521+
MOV_move(tdbb, &nextValue.vlu_desc, &impure->m_result.vlu_desc);
529522
}
530-
}
531-
else
532-
{
533-
auto result = impure->m_result.vlu_int128;
534-
const auto finish = impure->m_finish.vlu_int128;
535-
const auto step = impure->m_step.vlu_int128;
536-
537-
if (((step.sign() > 0) && (result.compare(finish) <= 0)) ||
538-
((step.sign() < 0) && (result.compare(finish) >= 0)))
523+
catch (const status_exception&)
539524
{
540-
Record* const record = request->req_rpb[m_stream].rpb_record;
541-
542-
auto toDesc = m_format->fmt_desc.begin();
543-
544-
dsc fromDesc;
545-
fromDesc.makeInt128(impure->m_scale, &result);
546-
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
547-
548-
result = result.add(step);
549-
impure->m_result.vlu_int128 = result;
550-
551-
return true;
525+
tdbb->tdbb_status_vector->clearException();
526+
// stop evaluate next result
527+
impure->m_stepSign = 0;
552528
}
529+
530+
return true;
553531
}
554532

555533
return false;

0 commit comments

Comments
 (0)