Skip to content

Conversation

@Noremos
Copy link
Contributor

@Noremos Noremos commented Nov 7, 2024

Hello.
Currently, it is possible to CREATE, ALTER, or RECREATE a PACKAGE

set autoterm;
CREATE PACKAGE TEST_PACKAGE
AS
BEGIN
    FUNCTION DUMMY() RETURNS INT;
END;

CREATE OR ALTER PACKAGE TEST_PACKAGE
AS
BEGIN
    FUNCTION DUMMY() RETURNS INT;
END;

ALTER PACKAGE TEST_PACKAGE
AS
BEGIN
    FUNCTION DUMMY() RETURNS INT;
END;

RECREATE PACKAGE TEST_PACKAGE
AS
BEGIN
    FUNCTION DUMMY() RETURNS INT;
END;

However, when it comes to a package body, some operations may fail:

set term ^;
ALTER PACKAGE BODY TEST_PACKAGE
AS
BEGIN
    FUNCTION DUMMY() RETURNS INT
    AS
    BEGIN
        RETURN 1;
    END
END^

This results in the following error message:

Statement failed, SQLSTATE = 42000
Dynamic SQL Error
-SQL error code = -104
-Token unknown - line 1, column 20
-TEST_PACKAGE

This is a really confusing error message. The problem is that the PACKAGE BODY does not support the ALTER or CREATE OR ALTER statements, yet the message references the package name, which can be misleading.

I think it is worth adding this parse rules to not confuse the users.
From my perspective, ALTER PACKAGE BODY is essentially equivalent to RECREATE statement, so I used the RECREATE node without changing the package code.

@Noremos Noremos changed the title Add ALTER PACKAGE and CRETE OR ALTER PACKAGE parse rules Add ALTER PACKAGE BODY and CRETE OR ALTER PACKAGE BODY parse rules Nov 7, 2024
@asfernandes
Copy link
Member

Please update doc/sql.extensions/README.packages.txt

src/dsql/parse.y Outdated
Jrd::SetDecFloatTrapsNode* setDecFloatTrapsNode;
Jrd::SetBindNode* setBindNode;
Jrd::SessionResetNode* sessionResetNode;
Jrd::RecreatePackageBodyNode* recreatePackageBodyNode;
Copy link
Member

Choose a reason for hiding this comment

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

Can this declaration be eliminated if you declare replace_package_body_clause to return a ddlNode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea

@asfernandes asfernandes merged commit 52657ab into FirebirdSQL:master Nov 8, 2024
24 checks passed
@dyemanov dyemanov added the rdb label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants