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

Problem with editing procedures being in use #7428

Closed
tomaszdubiel18 opened this issue Dec 17, 2022 · 54 comments
Closed

Problem with editing procedures being in use #7428

tomaszdubiel18 opened this issue Dec 17, 2022 · 54 comments

Comments

@tomaszdubiel18
Copy link

Hello.
Firebird 3.0.10 SuperServer.
We have one connection with nowait (isc_tpb_read_committed, isc_tpb_rec_version, isc_tpb_nowait).
We have second connection with wait (isc_tpb_read_committed, isc_tpb_rec_version, isc_tpb_wait).
Second connection executes create or alter procedure... - we add an another input parameter to an existing procedure, compile and commit.
Then we run select * from this_procedure from the first connection - when we run a procedure without a changes, we get an error:
"invalid request BLR at offset 160.
Input parameter mismatch for procedure..."
We should still be able to run the old version of procedure, without using a new parameter. Even if we try to execute a procedure with a new parameter, we get a similar error:
"Dynamic SQL Error.
Input parameter mismatch for procedure...".
Everything is ok, when we make changes to the main begin end block or to the output parameters. However, any change to the input parameters (adding, editing or deleting an input parameter) breaks the possibility of using that procedure for all already existing connections.

@tomaszdubiel18
Copy link
Author

Hello. Could you look into this problem?

@hvlad
Copy link
Member

hvlad commented Jan 9, 2023

Try to add input parameters at last position and with default value.
It should allow to run SP using old set of inputs.

@tomaszdubiel18
Copy link
Author

Yeah, thanks, I could try it, but it is still not a solution to the problem.

@dyemanov
Copy link
Member

dyemanov commented Jan 9, 2023

I can hardly see any bug in the engine. You change the declaration (contract) of the procedure and want the old version to be called from the already existing connections, while by design the new version becomes immediately visible. Your approach means that matching in the metadata cache should be based not just on name, but on the complete signature (name + parameters). This smells like a new feature to me.

@dyemanov
Copy link
Member

dyemanov commented Jan 9, 2023

Moreover, any new connection established right after the procedure was altered will never see its old version but it will be not prepared to deal with more input parameters either. This makes the whole effort somewhat pointless.

@tomaszdubiel18
Copy link
Author

tomaszdubiel18 commented Jan 9, 2023

Moreover, any new connection established right after the procedure was altered will never see its old version but it will be not prepared to deal with more input parameters either. This makes the whole effort somewhat pointless.

That's not true, Really. In nowait transaction I ran the procedure, in 2nd - wait - I added a new parameter. First connection is unable to run this procedure at all, but the 3rd connection started after the altering, no wait, launches the procedure with added parameter with completely no problem

@tomaszdubiel18
Copy link
Author

I can hardly see any bug in the engine. You change the declaration (contract) of the procedure and want the old version to be called from the already existing connections, while by design the new version becomes immediately visible. Your approach means that matching in the metadata cache should be based not just on name, but on the complete signature (name + parameters). This smells like a new feature to me.

Maybe it is by design, but do you really think this design is right? If you allow changing procedure input parameters in wait transaction, we assume it's not harmful, forbidden and we are safe to do it. Now doing this makes already connected users to forcely reconnect to be able to run the procedure at all. The stability drops.
How do you imagine implementing a change to the procedure being used in ERP system by many users? As far as I am concerned, there is even no way to find out which users use this procedure. In this situation the only solution is to shutdown the system.

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

That's why in big systems procedures are never changed. New procedures are created instead.

@tomaszdubiel18
Copy link
Author

That's how we deal with it now, but we seek for a much professional solution. We have to remember to drop every old procedure after adding a new one, so that the DB is not full of unused procedures.

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

You must not drop them. They are still needed for old applications which you don't want to break.

@tomaszdubiel18
Copy link
Author

That's not the case in our situation. We offer such a feature as user defined procedures being launched in specific places of the system, after given activities, as saving a document, etc. There is only one version of ERP system at time, there is no possibility to even start the system with the older version of the ERP system.

@dyemanov
Copy link
Member

dyemanov commented Jan 9, 2023

That's not true, Really. In nowait transaction I ran the procedure, in 2nd - wait - I added a new parameter. First connection is unable to run this procedure at all, but the 3rd connection started after the altering, no wait, launches the procedure with added parameter with completely no problem

I just tried and new connection expectedly raises "Input parameter mismatch" error. You cannot do select * from p (1) if P has two parameters.

@tomaszdubiel18
Copy link
Author

But why would the new connection execute the procedure with one parameter? The new connection was established after changing the procedure so it is natural it has to run with two parameters.

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

There is only one version of ERP system at time, there is no possibility to even start the system with the older version of the ERP system.

In this case your system is small and you should have no problem to upgrade database structure in the same technical downtime while you upgrade the application.

@tomaszdubiel18
Copy link
Author

tomaszdubiel18 commented Jan 9, 2023

There is only one version of ERP system at time, there is no possibility to even start the system with the older version of the ERP system.

In this case your system is small and you should have no problem to upgrade database structure in the same technical downtime while you upgrade the application.

We are the second main company mentioned on Firebird webpage in case studies :-)
You have to distinct between standard procedures, related to the ERP system - that cannot be at all modified by any user -
and between the user defined procedures, which is not the part of the standard ERP system. You can have as plenty of them as you can only think of.
Standard procedures can only be modified by the producer in the new version of ERP system.
User defined procedures can be added, modified and deleted at any time. They are not related to the ERP version.

@dyemanov
Copy link
Member

dyemanov commented Jan 9, 2023

But why would the new connection execute the procedure with one parameter? The new connection was established after changing the procedure so it is natural it has to run with two parameters.

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@tomaszdubiel18
Copy link
Author

But why would the new connection execute the procedure with one parameter? The new connection was established after changing the procedure so it is natural it has to run with two parameters.

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

The application should be able to run the procedures with the complete exact DLL at the time of establishing the connection.
When I connected at the time the procedure had only one parameter, until I disconnect I should be able run the procedure with only one parameter.
I reconnect - the procedure has already two parameters - so executing it with only one is a understood mistake. I should be able to run it with two parameters.
And that's the current situation - after reconnect everything is ok. But an old connection is no more able to run the procedure at all

@tomaszdubiel18
Copy link
Author

On 1/9/23 16:10, Dmitry Yemanov wrote: But why would the new connection execute the procedure with one parameter? The new connection was established after changing the procedure so it is natural it has to run with two parameters. How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?
Specially if that procedure is used by many people?

If that seems to be a problem for you, maybe it will be easier just to show in the error message the list of processes using given procedure?

@tomaszdubiel18
Copy link
Author

On 1/9/23 16:10, Dmitry Yemanov wrote: But why would the new connection execute the procedure with one parameter? The new connection was established after changing the procedure so it is natural it has to run with two parameters. How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?
Specially if that procedure is used by many people?

If that seems to be a problem for you, maybe it will be easier just to show in the error message the list of processes using given procedure?

I mean when we normally try to edit the procedure on read commited nowait and we can not do it because of users actively using it.

@tomaszdubiel18
Copy link
Author

Any solution would be appreciated. In current situation we either have to ask for the downtime to just implement a simple change in one user defined procedure or to create another one and, with several new procedures a week, the database will have crazy number of unused procedures which are useless and can only be a reason of problems.

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@dyemanov
Copy link
Member

dyemanov commented Jan 9, 2023

Currently, old version of the procedure is visible to the priorly prepared statements, but newly prepared statements will see the new version of the procedure. This is by design and this is how our metadata cache was working since the very beginning. And changing it to the new rules (keeping the old version visible until the connection ends) will likely to break way more applications that we may expect, because lots of users prefer to see the new procedure logic (i.e. its body) without reconnecting. Changing the body is much more common than changing the parameters.

@tomaszdubiel18
Copy link
Author

On 1/9/23 16:24, Tomasz Dubiel wrote: Any solution would be appreciated.
Something like disable any modification of procedures in use?
In current situation we either have to ask for the downtime to just implement a simple change in one user defined procedure or to create another one
Last not understood. Do you want to say that creation of new procedure affects already running transactions?
and, with several new procedures a week, the database will have crazy number of unused procedures which are useless and can only be a reason of problems.
Also not understood. Adding nightly job dropping unneeded procedures is simple task. I hope you do not have SO long running transactions ?

Let me remind you the main problem. I want to edit the procedure - Firebird gives me a message that it is unable to do it, because SOMEONE uses it. OK, then who uses it? Firebird doesnt tell me this. OK, but the change is urgent. What do I do? I call the customer to STOP THE WHOLE SYSTEM with users all around the country. This way I make the change fast enough.
I can also create a new procedure - but please understand, users can create as many of them and edit them as they like. In every moment. After adding a new procedure, when I forget about the old one, the database begins to have many unused procedures.

No, Im not saying anything like this that creation of new procedure affect already running transactions. At all.???

And the last one - we have hundreds of customers. Each one can have completely own procedures, doing exactly what the given customer needs. The needs in even one company can change over a course of time. There is no way to easily detect unwanted procedures, not used by anybody.

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

Can't we skip metadata cache during statement preparation and read procedure definition from system tables according to rules of the transaction used for that?..

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

I suppose you sugegst to do it in a case of some speicific errors
during statement preparation, yes?

No. Nowadays memory is big so we can have big page cache. Won't it work as effective as a metadata cache because system tables pass through it anyway?

@tomaszdubiel18
Copy link
Author

On 1/9/23 16:41, Tomasz Dubiel wrote: There is no way to easily detect unwanted procedures, not used by anybody.
Very simple. When you want to modify procedure but instead have to create the new one add the old procedure to the list of procedures to be deleted, that may be an SQL table for example.

That still requires extra work. I would expect from the RDBMS to be able to handle it in much more professional way.

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 9, 2023 via email

@hvlad
Copy link
Member

hvlad commented Jan 9, 2023

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

The application should be able to run the procedures with the complete exact DLL at the time of establishing the connection. When I connected at the time the procedure had only one parameter, until I disconnect I should be able run the procedure with only one parameter.

I doubt any DBMS have such a feature. Do you know one ?

Any solution would be appreciated.

Did you tries to use input params with default values ? It was introduced for more-or-less similar purposes.

@aafemt
Copy link
Contributor

aafemt commented Jan 9, 2023

the only thing one needs to do when using object from it is
increment use counter, even not atomic one

So metadata cache object is kinda reference-counted. What may be wrong is DDL transaction simply replace it with the new version? Those requests that keep own copies of old one would be able to continue using it, right?

@dyemanov
Copy link
Member

This is more or less how it works now. But Tomasz needs new requests of existing connections to use the old copy of the procedure.

@tomaszdubiel18
Copy link
Author

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

The application should be able to run the procedures with the complete exact DLL at the time of establishing the connection. When I connected at the time the procedure had only one parameter, until I disconnect I should be able run the procedure with only one parameter.

I doubt any DBMS have such a feature. Do you know one ?

Any solution would be appreciated.

Did you tries to use input params with default values ? It was introduced for more-or-less similar purposes.

I have no idea, but I would be very surprised to hear that adding a new input parameter in Oracle or MSSQL breaks the possibility of using the procedure for all existing connections. Is it really that way?
I tried with the default value and yeah, thanks, it works, but you know it has its limitations?
"Important: If you give a parameter a default value, all parameters coming after it must also get default values."
Do you guarantee it is really a stable solution for every usecase?
And besides that, sometimes it is simply impossible to give any default value as the parameter needs to be not null and have a concrete value.
A really good solution to this problem would be simply to be able for existing connections to use an old version of the procedure
or
I would expect at least an information in error message when trying to compile the procedure "procedure xxx is in use by: xx, xx, xx attachments". Then I can just manually delete the given attachments and I dont need to create a mess with SP or stop the production.

@tomaszdubiel18
Copy link
Author

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

The application should be able to run the procedures with the complete exact DLL at the time of establishing the connection. When I connected at the time the procedure had only one parameter, until I disconnect I should be able run the procedure with only one parameter.

I doubt any DBMS have such a feature. Do you know one ?

Any solution would be appreciated.

Did you tries to use input params with default values ? It was introduced for more-or-less similar purposes.

I have no idea, but I would be very surprised to hear that adding a new input parameter in Oracle or MSSQL breaks the possibility of using the procedure for all existing connections. Is it really that way?

I would like to stress this out, because it is really the case in Firebird now. When I modify the procedure being in use and add an input value without a default value, the existing attachment is no longer able to run the procedure at all - neither with the old set of inputs, nor with the new.
With old set of inputs:
"invalid request BLR at offset 160.
Input parameter mismatch for procedure..."
With new set of inputs:
"Dynamic SQL Error.
Input parameter mismatch for procedure...".

@hvlad
Copy link
Member

hvlad commented Jan 10, 2023

How should the running application know what to pass in the second parameter if one second ago it didn't know about its existence at all?

The application should be able to run the procedures with the complete exact DLL at the time of establishing the connection. When I connected at the time the procedure had only one parameter, until I disconnect I should be able run the procedure with only one parameter.

I doubt any DBMS have such a feature. Do you know one ?

Any solution would be appreciated.

Did you tries to use input params with default values ? It was introduced for more-or-less similar purposes.

I have no idea, but I would be very surprised to hear that adding a new input parameter in Oracle or MSSQL breaks the possibility of using the procedure for all existing connections. Is it really that way?

Everyone have its own problems. For example, in MSSQL you may drop table used in stored procedure and it (SP) will not run anymore. And nobody have idea about such broken SP until attempt to execute it. In Firebird this is impossible.

I tried with the default value and yeah, thanks, it works, but you know it has its limitations? "Important: If you give a parameter a default value, all parameters coming after it must also get default values."

Of course I know.

Do you guarantee it is really a stable solution for every usecase?

You asked for "any solution". I offer one that may be used immediately (no need to wait for a fix).
Now you want guarantee for every use case. Please, define what you really need ;)

And besides that, sometimes it is simply impossible to give any default value as the parameter needs to be not null and have a concrete value.

Set default value to NULL and make SP work "as before" when such param is NULL.

A really good solution to this problem would be simply to be able for existing connections to use an old version of the procedure

This returns us to the my initial question - do you know any DBMS that works this way ? I'm almost sure - no, it is not exists. And there is a reasons for it.

or I would expect at least an information in error message when trying to compile the procedure "procedure xxx is in use by: xx, xx, xx attachments". Then I can just manually delete the given attachments and I dont need to create a mess with SP or stop the production.

When SP is altered in no-wait transaction, Firebird already report "object in use" error. All you additionally need in this case: it is info - which attachments is uses this SP. This information could be obtained from lock manager, while it is not too easy. If this really helps you (I'm not sure), we can think in this direction...

@dyemanov
Copy link
Member

And now this is really a bug. PRC_dropped is set by AST but never tested by the other code. The same for functions.

@tomaszdubiel18
Copy link
Author

or I would expect at least an information in error message when trying to compile the procedure "procedure xxx is in use by: xx, xx, xx attachments". Then I can just manually delete the given attachments and I dont need to create a mess with SP or stop the production.

When SP is altered in no-wait transaction, Firebird already report "object in use" error. All you additionally need in this case: it is info - which attachments is uses this SP. This information could be obtained from lock manager, while it is not too easy. If this really helps you (I'm not sure), we can think in this direction...

Of course, that would be huge help. If I can't edit the procedure, because the problem is that someone uses it, then all I need to remove the reason is to know what attachments use that, so I can proceed.

@hvlad
Copy link
Member

hvlad commented Jan 10, 2023

Looks like it was fixed in fb4, see #6414 (yes, that ticket about another issue)

@tomaszdubiel18: could you try fb4 or fb5 ?
@asfernandes: could you comment it ? :)

@tomaszdubiel18
Copy link
Author

Is it about @dyemanov comment or my last comment?
And the fix can not land in fb3.0?

@hvlad
Copy link
Member

hvlad commented Jan 10, 2023

Is it about @dyemanov comment or my last comment?

It is about current ticket (issue), sorry to be not clear. I.e. it looks as fixed since v4.

And the fix can not land in fb3.0?

Needs investigation, can't say right now.

@asfernandes
Copy link
Member

I doubt any DBMS have such a feature. Do you know one ?

While I never used it and found it to have some difficulties, I think something like Oracle edition-based metadata to be useful in this type of scenario.

@asfernandes
Copy link
Member

@hvlad I suppose you were talking about checks of *_dropped flags.
It may help with DSQL, but what if usage of the changed object is in another cached and unmodified procedure?
Then DSQL layer is not in the game and I suppose same type of problem may happen.

I do not know what is right because all choices are very bad. It's first a conceptual problem before an implementation problem.
It's a general DBMS problem. Stored routines works good until you need to change them with others active connections.

@tomaszdubiel18
Copy link
Author

tomaszdubiel18 commented Jan 19, 2023

When SP is altered in no-wait transaction, Firebird already report "object in use" error. All you additionally need in this case: it is info - which attachments is uses this SP. This information could be obtained from lock manager, while it is not too easy. If this really helps you (I'm not sure), we can think in this direction...

Should I create another issue? Because @dyemanov confirmed here the bug with dropping objects and that what you mentioned, is something another, but very useful in my case.

@tomaszdubiel18
Copy link
Author

For the possibility to return attachments using given database object I created a separate topic: #7454
I would like to sum up everything so everyone has a clear view whats going on.
@hvlad, is the exact bug already confirmed here by @dyemanov fixed in FB 4.0? If yes, could be included in FB 3.0 as well?
For the time being, editing procedures, their body, output parameters, except for the input ones, should be ok?

@hvlad
Copy link
Member

hvlad commented Jan 20, 2023

@hvlad, is the exact bug already confirmed here by @dyemanov fixed in FB 4.0? If yes, could be included in FB 3.0 as well?

I already asked you to check FB4 with your real test case. I can check my artificial test only and it makes no guarantee for your real case. Or I can prepare build of FB3 with supposed fix for you. On success, we can discuss including it into FB3 with team.

For the time being, editing procedures, their body, output parameters, except for the input ones, should be ok?

Yes, looks likely.

@tomaszdubiel18
Copy link
Author

If you could, it would be easier for me to check on FB3 with fix than to install another Firebird server.

hvlad added a commit that referenced this issue Jan 20, 2023
@hvlad
Copy link
Member

hvlad commented Jan 20, 2023

Here it is Firebird-3.0.11.33654_x64.7z.zip
(It is 7z archive, but due to stupid gh limitation I've added ".zip" to the name)

@tomaszdubiel18
Copy link
Author

I'm not able to download this. Every time it gets around 5 or 6 MB out of 7 and the transfer stops.

@tomaszdubiel18
Copy link
Author

It went through finally, I will check.

@tomaszdubiel18
Copy link
Author

I execute procedure in one nowait transaction, in second wait transaction I add, modify, delete input and output parameters and when executing with the current set of input parameters, I get no errors. It looks it works correctly.

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

6 participants