Skip to content

Fix collate and null processing in package constants#9037

Open
Noremos wants to merge 5 commits into
FirebirdSQL:masterfrom
Noremos:constants_collate_and_null_fix
Open

Fix collate and null processing in package constants#9037
Noremos wants to merge 5 commits into
FirebirdSQL:masterfrom
Noremos:constants_collate_and_null_fix

Conversation

@Noremos
Copy link
Copy Markdown
Contributor

@Noremos Noremos commented May 25, 2026

Comment thread src/dsql/PackageNodes.epp Outdated
jrd_tra* transaction = dsqlScratch->getTransaction();
thread_db* tdbb = JRD_get_thread_data();
*desc = ConstantValue::getDesc(tdbb, transaction, m_fullName);
desc->setNullable(true);
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.

Why is constant references always marked as nullable?

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.

To return NULL from the ReferenceNode, the nullable flag is requered. It would be nice to explicitly check the not null state of the field, but as I can see, it does not stored in RDB$FIELD. So I decied to put the nullable flag withou extra checks.

The not null clasue can only be used when the type is a domain:

create domain my_domain int not null;

create or alter package my_package as
begin
    constant my_const my_domain = null;
end;

But in this case an error will appear instanly:

Statement failed, SQLSTATE = 2F000
validation error for CAST, value "*** null ***"
-Error while parsing BLR value of the constant "PUBLIC"."MY_PACKAGE"."MY_CONST"

So, putting nullable flag in any case is safe

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.

Every other kind of expressions, include composed ones, do return nullable flag correctly. I see no reason for constants to flag nullable incorrectly.

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.

It would be nice to explicitly check the not null state of the field, but as I can see, it does not stored in RDB$FIELD

RDB$FIELDS.RDB$NULL_FLAG ?

Copy link
Copy Markdown
Contributor Author

@Noremos Noremos May 26, 2026

Choose a reason for hiding this comment

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

It would be nice to explicitly check the not null state of the field, but as I can see, it does not stored in RDB$FIELD

RDB$FIELDS.RDB$NULL_FLAG ?

I missed that somehow. Fixed

Comment thread src/jrd/Package.epp Outdated
Comment thread src/jrd/Package.epp

const dsc* temp = EVL_expr(tdbb, request, static_cast<ValueExprNode*>(csb->csb_node));
EVL_make_value(tdbb, temp, &m_value, &getPool());
//! Execute it here, while AutoSetRestore2 is active
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
//! Execute it here, while AutoSetRestore2 is active
// Execute it here, while AutoSetRestore2 is active

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 used an exclamation point to highlight the warning. Is this style inappropriate for Firebird?

Comment thread src/jrd/Package.epp

void ConstantValue::executeCsbNode(thread_db* tdbb, CompilerScratch* csb)
{
ValueExprNode* exprNode = static_cast<ValueExprNode*>(csb->csb_node);
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.

Assert csb->csb_node && csb->csb_node->getKind() == DmlNode::KIND_VALUE.

Noremos and others added 2 commits May 26, 2026 15:39
Co-authored-by: Adriano dos Santos Fernandes <529415+asfernandes@users.noreply.github.com>
Also use constant instead of 0 for dtype check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants