Skip to content

New 'UNLIST' function #8418

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

Merged
merged 10 commits into from
Mar 24, 2025
Merged

New 'UNLIST' function #8418

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0167f8f
Implementation of the new internal function UNLIST
ChudaykinAlex Jan 28, 2025
0653686
Added raedme file
ChudaykinAlex Jan 29, 2025
c7c6429
Fix typo
ChudaykinAlex Jan 29, 2025
be29efd
Correction of comments from the review. Added initialization of input…
ChudaykinAlex Feb 3, 2025
dae3606
Improved names, added error output when blr code is incorrect
ChudaykinAlex Feb 5, 2025
4e4420c
Conversion to string_view
ChudaykinAlex Feb 13, 2025
c4928d5
Fix windows build
ChudaykinAlex Feb 19, 2025
f0a8d10
Changed processing of BLOB as input parameter.
ChudaykinAlex Mar 11, 2025
b55d5b2
Merge branch 'master' into master_unlist_new_function
ChudaykinAlex Mar 11, 2025
42bbb27
Corrected remarks about variables belonging to the base class. correc…
ChudaykinAlex Mar 12, 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
7 changes: 7 additions & 0 deletions src/jrd/recsrc/RecordSource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,9 @@ namespace Jrd
struct Impure : public RecordSource::Impure
{
RecordBuffer* m_recordBuffer;
blb* m_blob;
Copy link
Member

@asfernandes asfernandes Mar 11, 2025

Choose a reason for hiding this comment

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

Aren't you mixing specifics of UnlistFunctionScan's impure in the base TableValueFunctionScan?

Firebird::string* m_separatorStr;
Firebird::string* m_resultStr;
};

public:
Expand All @@ -1483,6 +1486,8 @@ namespace Jrd
void assignParameter(thread_db* tdbb, dsc* fromDesc, const dsc* toDesc, SSHORT toId,
Record* record) const;

virtual bool nextBuffer(thread_db* tdbb) const;

const Firebird::string m_alias;
};

Expand All @@ -1504,6 +1509,8 @@ namespace Jrd
void internalGetPlan(thread_db* tdbb, PlanEntry& planEntry, unsigned level,
bool recurse) const final;

bool nextBuffer(thread_db* tdbb) const final;

private:
NestConst<ValueListNode> m_inputList;
};
Expand Down
202 changes: 127 additions & 75 deletions src/jrd/recsrc/TableValueFunctionScan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "firebird.h"
#include "../dsql/ExprNodes.h"
#include "../dsql/StmtNodes.h"
#include "../jrd/align.h"
#include "../jrd/jrd.h"
#include "../jrd/mov_proto.h"
#include "../jrd/optimizer/Optimizer.h"
Expand All @@ -36,7 +35,7 @@ using namespace Firebird;
using namespace Jrd;

TableValueFunctionScan::TableValueFunctionScan(CompilerScratch* csb, StreamType stream,
const Firebird::string& alias)
const string& alias)
: RecordStream(csb, stream), m_alias(csb->csb_pool, alias)
{
m_impure = csb->allocImpure<Impure>();
Expand All @@ -60,6 +59,24 @@ void TableValueFunctionScan::close(thread_db* tdbb) const
delete impure->m_recordBuffer;
impure->m_recordBuffer = nullptr;
}

if (impure->m_blob)
{
impure->m_blob->BLB_close(tdbb);
impure->m_blob = nullptr;
}

if (impure->m_separatorStr)
{
delete impure->m_separatorStr;
impure->m_separatorStr = nullptr;
}

if (impure->m_resultStr)
{
delete impure->m_resultStr;
impure->m_resultStr = nullptr;
}
}
}

Expand All @@ -78,13 +95,19 @@ bool TableValueFunctionScan::internalGetRecord(thread_db* tdbb) const
}

rpb->rpb_number.increment();

if (!impure->m_recordBuffer->fetch(rpb->rpb_number.getValue(), rpb->rpb_record))
do
{
rpb->rpb_number.setValid(false);
return false;
}
return true;
if (!impure->m_recordBuffer->fetch(rpb->rpb_number.getValue(), rpb->rpb_record))
{
if (!nextBuffer(tdbb))
{
rpb->rpb_number.setValid(false);
return false;
}
continue;
}
return true;
} while (1);
Copy link
Member

@asfernandes asfernandes Mar 11, 2025

Choose a reason for hiding this comment

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

Suggested change
} while (1);
} while (true);

}

bool TableValueFunctionScan::refetchRecord(thread_db* /*tdbb*/) const
Expand All @@ -97,8 +120,7 @@ WriteLockResult TableValueFunctionScan::lockRecord(thread_db* /*tdbb*/) const
status_exception::raise(Arg::Gds(isc_record_lock_not_supp));
}

void TableValueFunctionScan::getLegacyPlan(thread_db* tdbb, Firebird::string& plan,
unsigned level) const
void TableValueFunctionScan::getLegacyPlan(thread_db* tdbb, string& plan, unsigned level) const
{
if (!level)
plan += "(";
Expand Down Expand Up @@ -132,6 +154,11 @@ void TableValueFunctionScan::assignParameter(thread_db* tdbb, dsc* fromDesc, con
memcpy(toDescValue.dsc_address, fromDesc->dsc_address, fromDesc->dsc_length);
}

bool TableValueFunctionScan::nextBuffer(thread_db* /*tdbb*/) const
{
return false;
Copy link
Member

@asfernandes asfernandes Mar 11, 2025

Choose a reason for hiding this comment

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

Should not TableValueFunctionScan be an abstract class instead?

}

UnlistFunctionScan::UnlistFunctionScan(CompilerScratch* csb, StreamType stream, const string& alias,
ValueListNode* list)
: TableValueFunctionScan(csb, stream, alias), m_inputList(list)
Expand All @@ -141,16 +168,8 @@ UnlistFunctionScan::UnlistFunctionScan(CompilerScratch* csb, StreamType stream,
void UnlistFunctionScan::internalOpen(thread_db* tdbb) const
{
const auto request = tdbb->getRequest();
const auto impure = request->getImpure<Impure>(m_impure);

impure->irsb_flags = irsb_open;

const auto rpb = &request->req_rpb[m_stream];

MemoryPool& pool = *tdbb->getDefaultPool();
impure->m_recordBuffer = FB_NEW_POOL(pool) RecordBuffer(pool, m_format);

Record* const record = VIO_record(tdbb, rpb, m_format, &pool);

rpb->rpb_number.setValue(BOF_NUMBER);

Expand All @@ -172,79 +191,39 @@ void UnlistFunctionScan::internalOpen(thread_db* tdbb) const
return;
}

const auto impure = request->getImpure<Impure>(m_impure);
impure->irsb_flags |= irsb_open;
impure->m_recordBuffer = FB_NEW_POOL(pool) RecordBuffer(pool, m_format);

Record* const record = VIO_record(tdbb, rpb, m_format, &pool);

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

const auto textType = toDesc->getTextType();

auto setStringToRecord = [&] (string& str, USHORT length = 0)
impure->m_separatorStr = FB_NEW_POOL(pool)
string(pool, MOV_make_string2(tdbb, separatorDesc, textType, true));

if (impure->m_separatorStr->isEmpty())
{
if (length == 0)
length = str.length();
const string valueStr(MOV_make_string2(tdbb, valueDesc, textType, false));
dsc fromDesc;
fromDesc.makeText(length, textType, (UCHAR*)str.c_str());
fromDesc.makeText(valueStr.size(), textType, (UCHAR*)(IPTR)(valueStr.c_str()));
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
impure->m_recordBuffer->store(record);
};

string separatorStr(MOV_make_string2(tdbb, separatorDesc, textType, true));

if (separatorStr.isEmpty())
{
string valueStr(MOV_make_string2(tdbb, valueDesc, textType, false));
setStringToRecord(valueStr);
return;
}

if (valueDesc->isBlob())
Copy link
Member

@asfernandes asfernandes Jan 30, 2025

Choose a reason for hiding this comment

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

Others Firebird functions compare strings using collation rules.
Here this is disregard. Should it be?

Copy link
Member

@dyemanov dyemanov Jan 31, 2025

Choose a reason for hiding this comment

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

Good question to be discussed. Theoretically, one may ask to split the text into chunks by e.g. both 'A' and 'a' used as a case-insensitive separator 'a'. But honestly, I cannot imagine that being used in practice. And collation-aware compares would add some extra overhead. So maybe it's worth comparing byte-wise (converting both value and separator to the common data type, if necessary). Or maybe we should consider two code branches depending on whether byte-wise compare is possible or not.

{
blb* blob = blb::open(tdbb, request->req_transaction,
reinterpret_cast<bid*>(valueDesc->dsc_address));

string resultStr;

while (!(blob->blb_flags & BLB_eof))
{
UCHAR buffer[BUFFER_LARGE];
const auto length = blob->BLB_get_data(tdbb, buffer, sizeof(buffer), false);
if (length == 0)
continue;

std::string_view separatorView(separatorStr.begin(), separatorStr.length());
std::string_view valueView(reinterpret_cast<char*>(buffer), length);

auto end = std::string_view::npos;
do
{
auto size = end = valueView.find(separatorView);
if (end == std::string_view::npos)
{
if (!valueView.empty())
resultStr.append(valueView.data(), valueView.length());

break;
}

if (size > 0)
resultStr.append(valueView.data(), size);

valueView.remove_prefix(size + separatorView.length());

if (resultStr.hasData())
{
setStringToRecord(resultStr);
resultStr.erase();
}
} while (end != std::string_view::npos);
}
// Tail
if (resultStr.hasData())
setStringToRecord(resultStr);
impure->m_blob = blb::open(tdbb, request->req_transaction,
reinterpret_cast<bid*>(valueDesc->dsc_address));
impure->m_resultStr = FB_NEW_POOL(pool) string(pool);
}
else
{
const string& separatorStr = *impure->m_separatorStr;
string valueStr(MOV_make_string2(tdbb, valueDesc, textType, true));

auto end = AbstractString::npos;
do
{
Expand All @@ -258,7 +237,12 @@ void UnlistFunctionScan::internalOpen(thread_db* tdbb) const
}

if (size > 0)
setStringToRecord(valueStr, size);
{
dsc fromDesc;
fromDesc.makeText(size, textType, (UCHAR*)(IPTR)(valueStr.c_str()));
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
impure->m_recordBuffer->store(record);
}

valueStr.erase(valueStr.begin(), valueStr.begin() + end + separatorStr.length());
} while (end != AbstractString::npos);
Expand All @@ -277,3 +261,71 @@ void UnlistFunctionScan::internalGetPlan(thread_db* tdbb, PlanEntry& planEntry,
planEntry.alias = m_alias;
}

bool UnlistFunctionScan::nextBuffer(thread_db* tdbb) const
{
const auto request = tdbb->getRequest();
const auto impure = request->getImpure<Impure>(m_impure);

if (impure->m_blob)
{
auto setStringToRecord = [&](const string& str)
{
Record* const record = request->req_rpb[m_stream].rpb_record;

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

dsc fromDesc;
fromDesc.makeText(str.length(), textType, (UCHAR*)(IPTR)(str.c_str()));
assignParameter(tdbb, &fromDesc, toDesc, 0, record);
impure->m_recordBuffer->store(record);
};

MoveBuffer buffer;
const auto address = buffer.getBuffer(MAX_COLUMN_SIZE);
const auto length = impure->m_blob->BLB_get_data(tdbb, address, MAX_COLUMN_SIZE, false);
if (length > 0)
{
const std::string_view separatorView(impure->m_separatorStr->data(),
impure->m_separatorStr->length());
std::string_view valueView(reinterpret_cast<char*>(address), length);
auto end = std::string_view::npos;
do
{
auto size = end = valueView.find(separatorView);
if (end == std::string_view::npos)
{
if (!valueView.empty())
impure->m_resultStr->append(valueView.data(), valueView.length());

break;
}

if (size > 0)
impure->m_resultStr->append(valueView.data(), size);

valueView.remove_prefix(size + separatorView.length());

if (impure->m_resultStr->hasData())
{
setStringToRecord(*impure->m_resultStr);
impure->m_resultStr->erase();
}
} while (end != std::string_view::npos);

return true;
}

if (impure->m_blob->blb_flags & BLB_eof)
{
if (impure->m_resultStr->hasData())
{
setStringToRecord(*impure->m_resultStr);
impure->m_resultStr->erase();
return true;
}
}
}

return false;
}