Skip to content

Package Constant: Check missing dsc_address instead of dsc_dtype and restore impure after using existing request #9065

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

Package Constant: Check missing dsc_address instead of dsc_dtype and restore impure after using existing request #9065

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
e93b31f
Check dsc_address missing instead of dsc_dtype and restore impure and…
Jun 16, 2026
a2aad64
Use correct way to use csb_impure
Jun 21, 2026
f92e411
Always use handmade stmt and request to make constant value
Jun 22, 2026
36595a3
Remove irrelevant Package Constant comments
Jun 22, 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/dsql/PackageNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ dsc* PackageReferenceNode::execute(thread_db* tdbb, Request* request) const
Package* package = m_package(request->getResources());
package->checkReload(tdbb);

dsc* constantDsc = package->findConstant(m_fullName)->makeValue(tdbb, request);
dsc* constantDsc = package->findConstant(m_fullName)->makeValue(tdbb);
if (constantDsc == nullptr || constantDsc->isNull())
return nullptr;

Expand Down
38 changes: 16 additions & 22 deletions src/jrd/Package.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,22 +168,23 @@ bid ConstantValue::getBlobId(thread_db* tdbb)
return m_blrBlobId.isEmpty() ? getConstantBid(tdbb, name) : m_blrBlobId;
}

dsc* ConstantValue::makeValue(thread_db* tdbb, Request* request)
dsc* ConstantValue::makeValue(thread_db* tdbb)
{
{
ReadLockGuard guard(m_makeValueLock, FB_FUNCTION);
if (m_value.vlu_desc.dsc_dtype != dtype_unknown)
if (m_value.vlu_desc.dsc_address != nullptr)

@asfernandes asfernandes Jun 23, 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.

Suggested change
if (m_value.vlu_desc.dsc_address != nullptr)
if (m_value.vlu_desc.dsc_address)

@aafemt aafemt Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using C-style implicit cast is generally bad idea in C++.

return &m_value.vlu_desc;
}

WriteLockGuard guard(m_makeValueLock, FB_FUNCTION);
if (m_value.vlu_desc.dsc_dtype != dtype_unknown) // Extra check in case of two callers waiting
if (m_value.vlu_desc.dsc_address != nullptr) // Extra check in case of two callers waiting

@asfernandes asfernandes Jun 23, 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.

Suggested change
if (m_value.vlu_desc.dsc_address != nullptr) // Extra check in case of two callers waiting
if (m_value.vlu_desc.dsc_address) // Extra check in case of two callers waiting

return &m_value.vlu_desc;

Attachment* attachment = tdbb->getAttachment();

MemoryPool* csb_pool = attachment->att_database->createPool();
Statement* handmadeStmt = nullptr;
Request* request = nullptr;
try
{
ContextPoolHolder context(tdbb, csb_pool);
Expand All @@ -195,28 +196,21 @@ dsc* ConstantValue::makeValue(thread_db* tdbb, Request* request)
bid constantBid = getBlobId(tdbb);

MET_parse_blob(tdbb, &name.schema, nullptr, &constantBid, &csb, nullptr, false, false);
fb_assert(csb->csb_node != nullptr);

if (request == nullptr)
{
// Came from Reload
handmadeStmt = Statement::makeStatement(tdbb, csb, true);
request = handmadeStmt->makeRootRequest(tdbb);
handmadeStmt = Statement::makeStatement(tdbb, csb, true);
request = handmadeStmt->makeRootRequest(tdbb);

AutoSetRestore2<Request*, thread_db> autoSetRequest(
tdbb, &thread_db::getRequest, &thread_db::setRequest, request);
AutoSetRestore2<Request*, thread_db> autoSetRequest(
tdbb, &thread_db::getRequest, &thread_db::setRequest, request);

request->setUsed();
request->setAttachment(attachment);
attachment->att_requests.add(request);
request->setUsed();
request->setAttachment(attachment);
attachment->att_requests.add(request);

TRA_attach_request(tdbb->getTransaction(), request);
TRA_attach_request(tdbb->getTransaction(), request);

// Execute it here, while AutoSetRestore2 is active
executeCsbNode(tdbb, csb);
}
else
executeCsbNode(tdbb, csb);
// Execute it while AutoSetRestore2 is active
executeCsbNode(tdbb, csb);
}
catch (const Exception& ex)
{
Expand Down Expand Up @@ -347,7 +341,7 @@ ScanResult Package::scan(thread_db* tdbb, ObjectBase::Flag flags)
skipMakeValue);

if (skipMakeValue)
this->m_callReload = true; // Value is necessary for to calculate hash
this->m_callReload = true;
}
END_FOR
}
Expand Down Expand Up @@ -426,7 +420,7 @@ ConstantValue& Package::addConstant(thread_db* tdbb,
auto& value = constants.add(constName, isPrivate);
value.updateValue(blrBlobId);
if (!skipMakeValue)
value.makeValue(tdbb, nullptr);
value.makeValue(tdbb);

return value;
}
Expand Down
3 changes: 2 additions & 1 deletion src/jrd/Package.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class ConstantValue final : public Firebird::PermanentStorage
delete m_value.vlu_string;
m_value = {};
m_value.vlu_desc = typeDesc;
m_value.vlu_desc.dsc_address = nullptr; // Make sure to call makeValue
}

void updateValue(const bid blobId)
Expand All @@ -82,7 +83,7 @@ class ConstantValue final : public Firebird::PermanentStorage

bid getBlobId(thread_db* tdbb);

dsc* makeValue(thread_db* tdbb, Request* request);
dsc* makeValue(thread_db* tdbb);

private:
void executeCsbNode(thread_db* tdbb, CompilerScratch* csb);
Expand Down
Loading