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
Inconsistencies with PSQL FUNCTION vs UDF [CORE5905] #6163
Comments
Modified by: @asfernandesassignee: Adriano dos Santos Fernandes [ asfernandes ] |
Commented by: @asfernandes Cannot reproduce. Please send full reproducible test case. |
Commented by: Maxim Kuzmin (cybermax) declare external function addDay -- CREATE OR ALTER PROCEDURE SP_ADDDAY (
END^ SET TERM ; ^ -- After this, PSQL-function ADDDAY have a 1 dependence. -- This operation is not defined for system tables. |
Commented by: Sean Leyne (seanleyne) I believe the correct handling of this issue is to prevent the new FUNCTION from being defined, since it's name collides with the existing/previously defined EXTERNAL FUNCTION. |
Commented by: @pavel-zotov It is 'ALTER FUNCTION' that is causes problem: -- doing on new DB: set term ^; alter function substrlen(input_str varchar(255), i smallint, n smallint) returns varchar(255) as select substrlen( '1234567890', 5, 4) from rdb$database; select rdb$get_context('USER_SESSION', 'WHO_WAS_INVOKED') as "was_psql_invoked ?" from rdb$database; -- Issues: was_psql_invoked ? Yes So, UDF became 'invisible' after ALTER FUNCTION was done. |
Commented by: Maxim Kuzmin (cybermax) Pavel Zotov, You're right, reason in ALTER. When use CREATE: Get an error: Description wrote from memory, and did not notice that in the original query was CREATE OR ALTER. |
Commented by: @asfernandes It seems people does not understood relation of UDFs and functions. It's perfectly valid to change a DECLARE'd function to a PSQL function using ALTER FUNCTION. |
Modified by: @pcisarstatus: Resolved [ 5 ] => Closed [ 6 ] |
Commented by: Maxim Kuzmin (cybermax) Adriano, I partially agree with you. Let the ALTER of UDF to PSQL-function is valid operation. But when executing DROP this function, it is no longer UDF, although text of message says it's UDF. |
Commented by: @asfernandes 1) That has nothing to do with the mix of DECLARE/FUNCTION too. |
Commented by: @pavel-zotov > PSQL function is not also an UDF (User Defined Function)? At least warning should be raised when we try to replace UDF with PSQL function, because this is implicit replacement of different DB object kinds. |
Commented by: Maxim Kuzmin (cybermax) Let's look in RDB$FUNCTIONS. SELECT 'RDB$FUNCTION_NAME: ' || COALESCE(F.RDB$FUNCTION_NAME, 'NULL') || ASCII_CHAR(13) || ASCII_CHAR(10) If UDF changed to PSQL, we have next data: If ADDDAY created at once as PSQL, we have: As you see, difference in RDB$LEGACY_FLAG. After ALTER, the change of type is not pure. Also, if ADDAY created as PSQL-function, execute query below: But ADDDAY is not external, it's PSQL function! 2) Historically, UDF - it's EXTERNAL function. Pre-3.0 UDF have an entry point and module name, written by c++, delphi and other language. Hiding from developer an internal core of function - external or PSQL - IMO, it's bad idea, which only confuses. Or cannot delete. I may not know how the function is declared, and for this knowledge it is necessary to look into the function. |
Commented by: @dyemanov From the engine perspective, UDF is *any* function, be it external or internal. Moreover, it can be external (backed by DLL) even if defined using the new syntax for functions (CREATE FUNCTION ... EXTERNAL NAME | EXTERNAL ENGINE) -- it's not UDF as you prefer to see it but still external. So I believe reporting all these cases as UDF (or just function) is OK. The question is whether changing from legacy UDF to new-style UDF should be allowed via ALTER. From one side, ALTER FUNCTION is allowed to change between PSQL and external (UDR) bodies, so it looks logical to apply this to legacy UDFs. But backward conversion PSQL->UDF is impossible this way. And we also have a separate ALTER EXTERNAL FUNCTION statement to change ENTRY_POINT | MODULE_NAME for legacy UDFs, that adds even more confusion. From another side, the current behaviour allows you to migrate from legacy UDFs (which are deprecated) to newer/safer alternatives (either PSQL-based or UDR) without need to care about dependencies which IMHO is a lovely bonus. |
Commented by: Maxim Kuzmin (cybermax) Dmitry Yemanov, Possible change type of function from old-style to new - it's greatly. But: I disagree with using word UDF and FUNCTION in different messages for one purpose. For select Unsuccessful execution caused by a system error that precludes successful execution of subsequent statements. Exception in function: Adriano, Bug with RDB$LEGACY_FLAG will be fixed? |
Commented by: @asfernandes I propose fix of RDB$LEGACY_FLAG if there is problem and change message "UDF" (reported in dependencies) to "Function". Surely there is a problem to revert from PSQL function to legacy UDF, but if for a long time it got unnoticed, I don't see a need to implement this now after legacy UDF has been officialy deprecated. |
Modified by: @asfernandessummary: Mixing INTERNAL FUNCTION and UDF => Inconsistencies with PSQL FUNCTION vs UDF |
Modified by: @asfernandesstatus: Reopened [ 4 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 4.0 Beta 1 [ 10750 ] Fix Version: 3.0.4 [ 10863 ] |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] QA Status: No test => Done successfully |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Closed [ 6 ] |
Submitted by: Maxim Kuzmin (cybermax)
In database created UDF "FORMAT_DATE". For test, I create INTERNAL FUNCTION FORMAT_DATE (introduced in FB 3.0):
CREATE FUNCTION FORMAT_DATE
RETURNS INTEGER
AS
BEGIN
RETURN 1;
END
After this, I can't use UDF - Firebird always use internal function.
Upon attempt drop internal function FORMAT_DATE, I get error:
DROP FUNCTION FORMAT_DATE
This operation is not defined for system tables.
unsuccessful metadata update.
cannot delete.
UDF FORMAT_DATE.
there are 294 dependencies.
And now I can not delete the test function.
Commits: b54ff50 a7e1624
The text was updated successfully, but these errors were encountered: