Skip to content

fix(gstat): Use correct RHD, RHDE and RHDF based on header flags in gstat #9245

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
TreeHunter9 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(gstat): Use correct RHD, RHDE and RHDF based on header flags in gstat #9245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a612e1b
fix(gstat): Use correct RHD, RHDE and RHDF based on header flags in g…
Oct 27, 2026
530f2ef
Add assert to check size of record
Oct 27, 2026
ec38d95
fix(sqz): Do not overrun input in `Compressor::truncateTail()` when e…
Oct 29, 2026
8a5c079
Fix assert to check size of record
Oct 29, 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
2 changes: 1 addition & 1 deletion src/jrd/BulkInsert.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void BulkInsert::Buffer::fragmentRecord(thread_db* tdbb, record_param* rpb, Comp
// Fill up data pages tail first until what's left fits on a single page.

auto size = dcc->getPackedLength();
fb_assert(size > max_data);
fb_assert(size > max_data - (RHDE_SIZE - RHD_SIZE));

do
{
Expand Down
2 changes: 1 addition & 1 deletion src/jrd/dpm.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4182,7 +4182,7 @@ static void store_big_record(thread_db* tdbb,
// Fill up data pages tail first until what's left fits on a single page.

auto size = dcc.getPackedLength();
fb_assert(size > max_data);
Comment thread
hvlad marked this conversation as resolved.
Resolved
fb_assert(size > max_data - (RHDE_SIZE - RHD_SIZE));

do
{
Expand Down
12 changes: 7 additions & 5 deletions src/jrd/sqz.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,18 @@ ULONG Compressor::truncateTail(ULONG outLength)
* Return the number of trailing input bytes that fit the given output length.
*
**************************************/
fb_assert(m_length > outLength);
[[maybe_unused]] const bool truncateAll = m_length <= outLength;

@hvlad hvlad Oct 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May be it is better to fix caller (store_big_record()) and not break compressor assumptions ?

@TreeHunter9 TreeHunter9 Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that we are breaking compressor assumption. As I see, it's fine that we can have head with no data, so at least outLength can be equal to m_length. But, anyway, I think it's fine that outLength can also be bigger then m_length because outLength is more like a buffer size, and return value will contain actual length anyway, so I see no restriction why we should limit outLength value to m_length (except branch if (m_runs.isEmpty()) at the beginning will break if outLength is bigger then m_length, but I can fix that).

@hvlad hvlad Oct 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dyemanov, what do you think ?

@dyemanov dyemanov Oct 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tend to agree that nothing is going to be broken.


ULONG inLength = 0;

if (m_runs.isEmpty())
{
m_length -= outLength;
return outLength;
inLength = MIN(outLength, m_length);
m_length -= inLength;
return inLength;
}

auto space = (int) outLength;
ULONG inLength = 0;

while (m_runs.hasData())
{
Expand Down Expand Up @@ -394,7 +396,7 @@ ULONG Compressor::truncateTail(ULONG outLength)
m_runs.pop();
}

fb_assert(m_runs.hasData());
fb_assert(truncateAll || m_runs.hasData());

if (m_allowUnpacked)
{
Expand Down
14 changes: 13 additions & 1 deletion src/utilities/gstat/dba.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,10 @@ static bool analyze_data_page( dba_rel* relation, const data_page* page, bool sw
relation->rel_record_space -= RHDF_SIZE;
relation->rel_record_space += analyze_fragments(relation, header);
}
else if (header->rhdf_flags & rhd_long_tranum)
{
relation->rel_record_space -= RHDE_SIZE;
}
else
{
relation->rel_record_space -= RHD_SIZE;
Expand Down Expand Up @@ -1663,8 +1667,14 @@ static ULONG analyze_fragments(dba_rel* relation, const rhdf* header)
if (!index->dpg_offset)
break;
space += index->dpg_length;
space -= RHDF_SIZE;

header = (const rhdf*) ((SCHAR *) page + index->dpg_offset);
if (header->rhdf_flags & rhd_incomplete)
space -= RHDF_SIZE;
else if (header->rhdf_flags & rhd_long_tranum)
space -= RHDE_SIZE;
else
space -= RHD_SIZE;
Comment thread
dyemanov marked this conversation as resolved.
Resolved

fragments++;
}
Expand Down Expand Up @@ -1868,6 +1878,8 @@ static ULONG analyze_versions( dba_rel* relation, const rhdf* header)
space -= RHDF_SIZE;
space += analyze_fragments(relation, header);
}
else if (header->rhdf_flags & rhd_long_tranum)
space -= RHDE_SIZE;
else
space -= RHD_SIZE;
}
Expand Down
Loading