"Dependencies broken" if trying to alter procedure used in a view

[mg-ica]

When trying to alter the body of a procedure which is used in a view i get an error message "Dependencies broken".

Using isql this works without any problem.

FB 2.5.9

[arvanus]

Can you provide a reproducible sample?

[mg-ica]

The following code causes this behavior every time. Without the intermediate COMMITs it's working

SET TERM ^ ;
CREATE PROCEDURE SP_TEST
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = '1';
  SUSPEND;
END
^
SET TERM ; ^

COMMIT;

CREATE VIEW VIEW_TEST(Y)
AS
SELECT (SELECT X FROM SP_TEST) AS Y FROM RDB$DATABASE;

COMMIT;

SET TERM ^ ;
ALTER PROCEDURE SP_TEST
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = '2';
  SUSPEND;
END
^
SET TERM ; ^

[arvanus]

This?

[mg-ica]

Exactly that.

[arvanus]

Not exactly a bug, more to a friendly warning when you are committing the transaction that you have pending dependencies and need to take a look at than.

[mg-ica]

Even though this is just a warning, it's very annoying. Especially when it pops up multiple times while deploying a longer script. For example, when an update script is distributed to colleagues, everyone needs to know that they can and should ignore this warning. I think it's a false warning. After all, only the body was changed, and without COMMITs, the warning wouldn't appear.

[baldeuniversel]

Even though this is just a warning, it's very annoying. Especially when it pops up multiple times while deploying a longer script. For example, when an update script is distributed to colleagues, everyone needs to know that they can and should ignore this warning. I think it's a false warning. After all, only the body was changed, and without COMMITs, the warning wouldn't appear.

Take a look here (as @arvanus said) ->

"... that you have pending dependencies ..."

[mg-ica]

The warning is very helpful when the input/output parameters of a procedure change (unintentionally), but if only the body changes, the warning remains annoying and pointless in my opinion. The word "warning" would also be very useful in the popup. It would also be a hint that there "might" be a problem.

[baldeuniversel]

This?

@mg-ica ! The logic of this code is quite confusing 🧐.

The error message you see in FlameRobin :

⚠️ Dependencies broken  
Some other procedures depend on SP_TEST:  
Procedure VIEW_TEST depends on parameter Y.

means (unless I'm mistaken ) that you're trying to modify a stored procedure (SP_TEST) that is used by a view (VIEW_TEST), and this modification breaks the dependency ~ specifically because SP_TEST no longer accepts or recognizes the parameter Y.

firebird (and therefore flamerobin) handles dependencies strictly. If you have a view or another procedure that calls SP_TEST with a parameter (Y in this case), and you modify SP_TEST without keeping that parameter, then the view VIEW_TEST can no longer function properly.

In your code :

CREATE OR ALTER PROCEDURE SP_TEST
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = '1';
  SUSPEND;
END

you aren’t declaring any input parameter (Y), but your view (VIEW_TEST) seems to be using a previous version of SP_TEST (or other 🤔 ) that expected a Y parameter.

[mg-ica]

Neither the first nor the second version of SP_TEST has an input parameter. Both versions only return a VARCHAR(8). The only difference is that the return value in the body is now set to '2' instead of '1'. (Y) is the column name of the view VIEW_TEST. VIEW_TEST is incorrectly referred to as a "procedure" in the warning.

[arvanus]

So, I found the commit https://github.com/mariuz/flamerobin/commit/f5ceb75b34e7e050eaf0f712bcb6fe43573f1c61

Warn user when he's about to encounter the FB bug CORE-1592 svn path=/trunk/flamerobin/; revision=1542

https://firebirdsql.org/en/report-core-dsql-201905/

Fixed CORE-1592 - Altering procedure parameters can lead to unrestorable database. https://github.com/FirebirdSQL/firebird/issues/2013 looks fixed at Fb4 What I can do is check the FB version. If it's older than 4, I’ll keep the warning. If it’s 4 or newer, maybe the warning can be skipped — but I’m still cautious about removing it entirely for user safety.

[baldeuniversel]

The warning is very helpful when the input/output parameters of a procedure change (unintentionally), but if only the body changes, the warning remains annoying and pointless in my opinion. The word "warning" would also be very useful in the popup. It would also be a hint that there "might" be a problem.

Here :

reCREATE VIEW VIEW_TEST(Y)
AS
SELECT (SELECT X FROM SP_TEST) AS Y FROM RDB$DATABASE;

The Y in parentheses right after the view name (VIEW_TEST(Y)) is not a parameter declaration, but a sort of column(s) returned by the view.

---

Which is what's causing the issue here (potentially), you're doing :

SELECT X FROM SP_TEST

however, SP_TEST is a procedure that takes no parameters here, while the name of the returned column (Y) in the view (VIEW_TEST) does not match X.

But more importantly, the system could think you're trying to rename X to Y, while calling a procedure that might (in the past or not) have expected Y as a parameter (or not). Result -> there's a dependency conflict ... .

[mg-ica]

SP_TEST returns X not Y.

Flamerobin could check the parameters of the procedure before and after the change and only if these change should the warning be issued.

[baldeuniversel]

SP_TEST returns X not Y.

Flamerobin could check the parameters of the procedure before and after the change and only if these change should the warning be issued.

Yep, I'm not saying otherwise, I'm just wondering, among other things, if there aren't any conflicts in the code itself (based on the last point I mentioned). If I had access to the database, I could run more informed tests.

I guess, you're doing something like this (this is an obvious example of the warning you have) :

-- Step 1: Create a procedure with a parameter
SET TERM ^ ;

CREATE OR ALTER PROCEDURE SP_TEST(Y VARCHAR(10))
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = 'Hello ' || Y;
  SUSPEND;
END^

SET TERM ; ^
COMMIT;

-- Step 2: Create a view that depends on **SP_TEST** (by calling **SP_TEST** with a constant *Y* parameter)
RECREATE VIEW VIEW_TEST(Y)
AS
SELECT (SELECT X FROM SP_TEST('World')) AS Y FROM RDB$DATABASE;

COMMIT;

-- Step 3: Now let's modify **SP_TEST** to remove the parameter (This will inevitably break the dependency with the **VIEW_TEST** view)

SET TERM ^ ;

CREATE OR ALTER PROCEDURE SP_TEST
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = 'Just X';
  SUSPEND;
END^

SET TERM ; ^
COMMIT;

This code should inevitably raise an alert message.

---

To avoid this warning message, I would have recommended doing it this way instead :

-- Step 1: Delete the view that depends on **SP_TEST**
DROP VIEW VIEW_TEST;
COMMIT;

-- Step 2: Modify the **SP_TEST** procedure freely (without parameters here)
SET TERM ^ ;

CREATE OR ALTER PROCEDURE SP_TEST
RETURNS (X VARCHAR(8))
AS
BEGIN
  X = 'Just X';
  SUSPEND;
END^

SET TERM ; ^
COMMIT;

-- Step 3: Recreate the **VIEW_TEST** view correctly, without calling any parameters
RECREATE VIEW VIEW_TEST(Y)
AS
SELECT X AS Y FROM SP_TEST;

COMMIT;

With this approach, there should be no problems 🤔.

Explanation regarding the view VIEW_TEST(Y) (tag[v0]) :

• We create or replace the view VIEW_TEST

• This view will expose a single column, here called Y

The name Y here is the name seen by SELECT queries on the view (it’s not a parameter).

Then, by doing the following (tag[v0]) :

SELECT X AS Y FROM SP_TEST;

• We call the procedure SP_TEST (with no parameter), which returns a field X ( defined in RETURNS (X VARCHAR(8)) )

• We rename this field X to Y using AS Y, so it matches the column name declared in the view (VIEW_TEST(Y)).

[mg-ica]

@baldeuniversel This is the example code which should not raise a warning: https://github.com/mariuz/flamerobin/issues/409#issuecomment-2673776506

There might be views which depend on views ... which need to be dropped to prevent this warning before a procedure body can be changed. This is no good solution.

[baldeuniversel]

@baldeuniversel This is the example code which should not raise a warning: #409 (comment)

There might be views which depend on views ... which need to be dropped to prevent this warning before a procedure body can be changed. This is no good solution.

I do not see where is the problem with this template of view 🧐 🤔.

It might not be the best approach in every situation. But in this specific sequence, this piece of code doesn’t seem out of place to me 🧐.

Sorry if my suggestion regarding you wasn’t one of the more anticipated/expected solutions 🔰.