Skip to content

Surgical port of core logic improvements: Date, DSQL, and Sort #8978

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
laBobberto wants to merge 4 commits into
base: master
Choose a base branch
from
+107 −124
Open

Surgical port of core logic improvements: Date, DSQL, and Sort #8978

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8cea0c0
Port logic improvements: date calculation, DSQL validation, and sort …
laBobberto Apr 2, 2026
89048d6
refactor`sort.cpp`:
laBobberto Apr 4, 2026
ea74d68
Replacing bitwise operations in `int NoThrowTimeStamp::convertGregori…
laBobberto Apr 4, 2026
f8402ca
Merge branch 'master' into fix_logic_surgical
laBobberto Apr 4, 2026
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
51 changes: 18 additions & 33 deletions src/common/classes/NoThrowTimeStamp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const ISC_TIME NoThrowTimeStamp::POW_10_TABLE[] =
NoThrowTimeStamp NoThrowTimeStamp::getCurrentTimeStamp(const char** error) noexcept
{
if (error)
*error = NULL;
*error = nullptr;
NoThrowTimeStamp result;

// NS: We round generated timestamps to whole millisecond.
Expand Down Expand Up @@ -341,51 +341,36 @@ int NoThrowTimeStamp::convertGregorianDateToWeekDate(const struct tm& times) noe
{
// Algorithm for Converting Gregorian Dates to ISO 8601 Week Date by Rick McCarty, 1999
// http://personal.ecu.edu/mccartyr/ISOwdALG.txt

const int y = times.tm_year + 1900;
const int dayOfYearNumber = times.tm_yday + 1;
const int dayOfYear = times.tm_yday + 1;

// Find the jan1Weekday for y (Monday=1, Sunday=7)
const int yy = (y - 1) % 100;
const int c = (y - 1) - yy;
const int g = yy + yy / 4;
const int y_1 = y - 1;
const int yy = y_1 % 100;
const int c = y_1 - yy;
const int g = yy + (yy / 4);
const int jan1Weekday = 1 + (((((c / 100) % 4) * 5) + g) % 7);

// Find the weekday for y m d
const int h = dayOfYearNumber + (jan1Weekday - 1);
const int weekday = 1 + ((h - 1) % 7);
const int weekday = 1 + ((dayOfYear + jan1Weekday - 2) % 7);

// Find if y m d falls in yearNumber y-1, weekNumber 52 or 53
int yearNumber, weekNumber;

if ((dayOfYearNumber <= (8 - jan1Weekday)) && (jan1Weekday > 4))
if ((dayOfYear <= (8 - jan1Weekday)) & (jan1Weekday > 4))
Copy link
Copy Markdown
Member

@AlexPeshkoff AlexPeshkoff Apr 3, 2026

Choose a reason for hiding this comment

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

Changing logical && to bitwise & here is plain bug.

Copy link
Copy Markdown
Author

@laBobberto laBobberto Apr 4, 2026

Choose a reason for hiding this comment

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

You're right, Alex. It was a mistake. Thanks for noticing, I've corrected it and your comment below.

{
yearNumber = y - 1;
weekNumber = ((jan1Weekday == 5) || ((jan1Weekday == 6) &&
isLeapYear(yearNumber))) ? 53 : 52;
const int prevYearLeap = (!(y_1 % 4) && ((y_1 % 100) || !(y_1 % 400)));
const int is53 = (jan1Weekday == 5) | ((jan1Weekday == 6) & prevYearLeap);
Copy link
Copy Markdown
Member

@AlexPeshkoff AlexPeshkoff Apr 3, 2026

Choose a reason for hiding this comment

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

Are you sure that operator | always returns 1 (assigned to int is53), not for example -1?

return 52 + is53;
}
else
{
yearNumber = y;

// Find if y m d falls in yearNumber y+1, weekNumber 1
const int i = isLeapYear(y) ? 366 : 365;

if ((i - dayOfYearNumber) < (4 - weekday))
{
yearNumber = y + 1;
weekNumber = 1;
}
// Find if y m d falls in yearNumber y+1, weekNumber 1
const int daysInYear = 365 + (!(y % 4) && ((y % 100) || !(y % 400)));
if ((daysInYear - dayOfYear) < (4 - weekday))
{
return 1;
}

// Find if y m d falls in yearNumber y, weekNumber 1 through 53
if (yearNumber == y)
{
const int j = dayOfYearNumber + (7 - weekday) + (jan1Weekday - 1);
weekNumber = j / 7;
if (jan1Weekday > 4)
weekNumber--;
}
const int j = dayOfYear + (7 - weekday) + (jan1Weekday - 1);
const int weekNumber = (j / 7) - (jan1Weekday > 4); // Subtract 1 if jan1Weekday > 4

return weekNumber;
}
Expand Down
53 changes: 26 additions & 27 deletions src/jrd/sort.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ void Sort::sort(thread_db* tdbb)
**************************************/
run_control* run;
merge_control* merge;
merge_control* merge_pool;

try
{
Expand Down Expand Up @@ -488,15 +487,8 @@ void Sort::sort(thread_db* tdbb)
{
fb_assert(!m_merge_pool); // shouldn't have a pool
m_merge_pool = FB_NEW_POOL(m_owner->getPool()) merge_control[count - 1];
merge_pool = m_merge_pool;
merge_control* merge_pool = m_merge_pool;
memset(merge_pool, 0, (count - 1) * sizeof(merge_control));
}
else
{
// Merge of 1 or 0 runs doesn't make sense
fb_assert(false); // We really shouldn't get here
merge = (merge_control*) *streams; // But if we do...
}

// Each pass through the vector builds a level of the merge tree
// by condensing two runs into one.
Expand Down Expand Up @@ -528,16 +520,23 @@ void Sort::sort(thread_db* tdbb)
(*m1)->rmh_parent = merge;
merge->mrg_stream_b = *m1++;

merge->mrg_record_a = NULL;
merge->mrg_record_b = NULL;
merge->mrg_record_a = nullptr;
merge->mrg_record_b = nullptr;

*m2++ = (run_merge_hdr*) merge;
count -= 2;
}

if (count)
*m2++ = *m1++;
count = m2 - streams;
*m2++ = *m1++;
count = m2 - streams;
}
}
else
{
// Merge of 1 or 0 runs doesn't make sense
fb_assert(false); // We really shouldn't get here
merge = (merge_control*) *streams; // But if we do...
}

streams.reset();
Expand Down Expand Up @@ -1272,7 +1271,7 @@ sort_record* Sort::getMerge(merge_control* merge)
}
else if ( (record = merge->mrg_record_a) )
{
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
merge = merge->mrg_header.rmh_parent;
}
else
Expand All @@ -1287,7 +1286,7 @@ sort_record* Sort::getMerge(merge_control* merge)
if (!merge->mrg_record_a)
{
record = merge->mrg_record_b;
merge->mrg_record_b = NULL;
merge->mrg_record_b = nullptr;
merge = merge->mrg_header.rmh_parent;
continue;
}
Expand All @@ -1311,7 +1310,7 @@ sort_record* Sort::getMerge(merge_control* merge)
(const UCHAR*) merge->mrg_record_b,
m_dup_callback_arg))
{
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
diddleKey((UCHAR*) merge->mrg_record_b, true, true);
continue;
}
Expand All @@ -1330,12 +1329,12 @@ sort_record* Sort::getMerge(merge_control* merge)
if (p[-1] < q[-1])
{
record = merge->mrg_record_a;
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
}
else
{
record = merge->mrg_record_b;
merge->mrg_record_b = NULL;
merge->mrg_record_b = nullptr;
}

merge = merge->mrg_header.rmh_parent;
Expand Down Expand Up @@ -1619,8 +1618,8 @@ void Sort::mergeRuns(USHORT n)
(*m1)->rmh_parent = merge;
merge->mrg_stream_b = *m1++;

merge->mrg_record_a = NULL;
merge->mrg_record_b = NULL;
merge->mrg_record_a = nullptr;
merge->mrg_record_b = nullptr;
*m2++ = (run_merge_hdr*) merge;
merge++;
count -= 2;
Expand Down Expand Up @@ -2267,8 +2266,8 @@ void PartitionedSort::buildMergeTree()
(*m1)->rmh_parent = m_merge;
m_merge->mrg_stream_b = *m1++;

m_merge->mrg_record_a = NULL;
m_merge->mrg_record_b = NULL;
m_merge->mrg_record_a = nullptr;
m_merge->mrg_record_b = nullptr;

*m2++ = (run_merge_hdr*)m_merge;
count -= 2;
Expand Down Expand Up @@ -2366,7 +2365,7 @@ sort_record* PartitionedSort::getMerge()
}
else if ((record = merge->mrg_record_a))
{
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
merge = merge->mrg_header.rmh_parent;
}
else
Expand All @@ -2381,7 +2380,7 @@ sort_record* PartitionedSort::getMerge()
if (!merge->mrg_record_a)
{
record = merge->mrg_record_b;
merge->mrg_record_b = NULL;
merge->mrg_record_b = nullptr;
merge = merge->mrg_header.rmh_parent;
continue;
}
Expand All @@ -2408,7 +2407,7 @@ sort_record* PartitionedSort::getMerge()
(const UCHAR*)merge->mrg_record_b,
aSort->m_dup_callback_arg))
{
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
aSort->diddleKey(rec_b, true, true);
continue;
}
Expand All @@ -2426,12 +2425,12 @@ sort_record* PartitionedSort::getMerge()
if (p[-1] < q[-1])
{
record = merge->mrg_record_a;
merge->mrg_record_a = NULL;
merge->mrg_record_a = nullptr;
}
else
{
record = merge->mrg_record_b;
merge->mrg_record_b = NULL;
merge->mrg_record_b = nullptr;
}

merge = merge->mrg_header.rmh_parent;
Expand Down