Skip to content
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

Comments to SP parameter (typed within SP header declaration) can be lost in metadata extraction script (if parameter is declared *without* DEFAULT or "=" modifier) [CORE6530] #6757

Open
firebird-automations opened this issue Apr 2, 2021 · 3 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: @pavel-zotov

Attachments:
missed-comments-to-sp-parameters-when-extract-metadata.7z

Consider script:

set bail on;
shell del C:\temp\tmp.fdb 2>nul;
create database 'localhost:C:\temp\tmp.fdb';
set term ^;
create or alter procedure sp_1(
p00 int -- single-line comment for `p00`
,p01 int
/*
multi-line comment
for `p01`,
typed on
several
lines
*/
,p02 int /* multi-line comment for `p02`, typed in one line */
,p03 varchar(1) character set utf8 -- single line comment for `p03` which has `character set` clause
,p04 type of column rdb$database.rdb$relation_id /* multi-line comment for `p04` which has `type of column` clause */
,
p05 int default null -- p05
-- ^^^^^^^
,
p06 int = null /* p06 */
-- ^
) returns (
r01 int -- r01
,r02 char(1) not null /* r02 */
)
as
begin
-- nop --
end
^
set term ;^
commit;

(it can be seen more readable in attached .7z / .png).

If we run 'isql -x' for this DB then extracted metadata SQL will be like this:

SET TERM ^ ;

/* Stored procedures headers */
CREATE OR ALTER PROCEDURE SP_1 (P00 INTEGER,
P01 INTEGER,
P02 INTEGER,
P03 VARCHAR(1) CHARACTER SET UTF8,
P04 TYPE OF COLUMN RDB$DATABASE.RDB$RELATION_ID,
P05 INTEGER default null -- p05
-- ^^^^^^^
,
P06 INTEGER = null /* p06 */
-- ^
)
RETURNS (R01 INTEGER,
R02 CHAR(1) CHARACTER SET NONE NOT NULL)
AS
BEGIN EXIT; END ^

SET TERM ; ^
COMMIT WORK;
SET AUTODDL ON;

COMMIT WORK;
SET AUTODDL OFF;
SET TERM ^ ;

/* Stored procedures bodies */

ALTER PROCEDURE SP_1 (P00 INTEGER,
P01 INTEGER,
P02 INTEGER,
P03 VARCHAR(1) CHARACTER SET UTF8,
P04 TYPE OF COLUMN RDB$DATABASE.RDB$RELATION_ID,
P05 INTEGER default null -- p05
-- ^^^^^^^
,
P06 INTEGER = null /* p06 */
-- ^
)
RETURNS (R01 INTEGER,
R02 CHAR(1) CHARACTER SET NONE NOT NULL)
AS
begin
-- nop --
end ^

SET TERM ; ^

So, comments for any input parameter will be lost if this pareameter was declared WITHOUT 'default' or '=' modifier.
Comments for output parameters will be lost at all.

Checked on:
WI-V4.0.0.2399
WI-V3.0.8.33431

FB 2.5 can not be used here: it does not preserve any comments of this type.

PS.
Priority was set to minor. This "feature" can be considered as inconvenience rather than real bug.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

Attachment: missed-comments-to-sp-parameters-when-extract-metadata.7z [ 13574 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @mrotteveel

I don't think this is a bug, as stored procedure parameters are reconstructed from the stored metadata, only the body itself is stored, so any comments put on parameters like this shouldn't survive. If anything could be considered a bug, it is the fact that the comment is preserved, although I think this is an accident of implementation.

I guess the reason it is preserved is that it is stored as part of the default value source (RDB$DEFAULT_SOURCE). If people need comments on procedure parameters, they should use `COMMENT ON` (see https://firebirdsql.org/file/documentation/html/en/refdocs/fblangref30/firebird-30-language-reference.html#fblangref30-ddl-comment-create), we shouldn't introduce another way to add comments.

@firebird-automations
Copy link
Collaborator Author

Commented by: @aafemt

Why not? Comments fit to comments better than to default source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant