Skip to content

Add ALTER PACKAGE BODY and CRETE OR ALTER PACKAGE BODY parse rules #8309

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.

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

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
@@ -863,6 +863,7 @@ using namespace Firebird;
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