[FB4] An improved support of UDF SCALAR_ARRAY-arguments #7845
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
It is the implementation of the second solution to issue FirebirdSQL#7843. 1. Server generates UDF argument names and saves them in RDB$ARGUMENT_NAME column. Structure of name: ARG<number of input argument>. src\dsql\DdlNodes.epp - CreateAlterFunctionNode::dsqlPass 2. Server provides client with name of UDF and SCALAR_ARRAY argument those are linked with DSQL parameter: select UDF(?) from rdb$database. work\src\dsql\ExprNodes.cpp - UdfCallNode::dsqlPass
dmitry-lipetsk commented Nov 15, 2023
src/dsql/DdlNodes.epp Outdated
| | ||
| fb_assert(!parameter->name.hasData()); | ||
| | ||
| if (this->returnType == NULL && this->udfReturnPos == (i + 1)) |
Contributor Author
There was a problem hiding this comment.
I am not sure that the condition 'this->returnType == NULL' is required here.
CreateAlterFunctionNode::dsqlPass: - we will use only udfReturnPos for detection PARAMETER that is marked as return-value - debug checks were added --- Database with different test UDFs (8246) is created without problems.
dmitry-lipetsk commented Nov 15, 2023
Comment on lines +13171 to +13181
| if (udfArg.desc.dsc_dtype != dtype_array) | ||
| { | ||
| // It is not array and will be ignored. | ||
| } | ||
| else | ||
| if (!node->dsqlFunction->udf_name.package.isEmpty()) | ||
| { | ||
| // We should complain here in the future! The parameter is | ||
| // out of bounds or the function doesn't declare input params. | ||
| // We can't provide with the exact information about package objects. | ||
| } | ||
| else | ||
| if (nodeArg->getType() == ExprNode::TYPE_PARAMETER) |
Contributor Author
There was a problem hiding this comment.
If you want, I will move these conditions into standalone method.
Contributor Author
| Test database with generated names for UDF input-arguments. Number of UDFs - 9905. |
Contributor Author
| SQL for check the names of argument (in IBP_TEST_FB40_D3) /*check argument positions*/ select 'arg pos', T0.RDB$FUNCTION_NAME as FUNC, T0.RDB$ARGUMENT_NAME as ARG_NAME from RDB$FUNCTION_ARGUMENTS T0 where T0.RDB$ARGUMENT_POSITION is NULL union all /*check udf input-argument names*/ select 'udf in-arg', T1.FUNC, T1.ARG_NAME from (select count(*) N, x.RDB$FUNCTION_NAME FUNC, x.RDB$ARGUMENT_NAME ARG_NAME from RDB$FUNCTIONS f join RDB$FUNCTION_ARGUMENTS x join RDB$FUNCTION_ARGUMENTS x2 on x.RDB$FUNCTION_NAME=x2.RDB$FUNCTION_NAME and x.RDB$ARGUMENT_POSITION>=x2.RDB$ARGUMENT_POSITION and (x.RDB$PACKAGE_NAME=x2.RDB$PACKAGE_NAME or x.RDB$PACKAGE_NAME is null and x2.RDB$PACKAGE_NAME is null) on f.RDB$FUNCTION_NAME=x.RDB$FUNCTION_NAME and (f.RDB$PACKAGE_NAME=x.RDB$PACKAGE_NAME or f.RDB$PACKAGE_NAME is null and x.RDB$PACKAGE_NAME is null) where coalesce(f.RDB$RETURN_ARGUMENT,0)<>x.RDB$ARGUMENT_POSITION and coalesce(f.RDB$RETURN_ARGUMENT,0)<>x2.RDB$ARGUMENT_POSITION and f.RDB$LEGACY_FLAG=1 group by x.RDB$FUNCTION_NAME, x.RDB$ARGUMENT_POSITION, x.RDB$ARGUMENT_NAME order by x.RDB$FUNCTION_NAME, x.RDB$ARGUMENT_POSITION, x.RDB$ARGUMENT_NAME) as T1 where ('ARG'||T1.N)<>coalesce(T1.ARG_NAME,'') union all /*check stored function argument names*/ select 'sf in-arg', T2.FUNC, T2.ARG_NAME from (select x.RDB$FUNCTION_NAME FUNC, x.RDB$ARGUMENT_NAME ARG_NAME from RDB$FUNCTIONS f join RDB$FUNCTION_ARGUMENTS x on f.RDB$FUNCTION_NAME=x.RDB$FUNCTION_NAME and (f.RDB$PACKAGE_NAME=x.RDB$PACKAGE_NAME or f.RDB$PACKAGE_NAME is null and x.RDB$PACKAGE_NAME is null) where coalesce(f.RDB$RETURN_ARGUMENT,0)<>x.RDB$ARGUMENT_POSITION and f.RDB$LEGACY_FLAG=0) T2 where coalesce(T2.ARG_NAME,'')=''Database is OK, when this query returns an empty result set. |
dmitry-lipetsk added a commit to dmitry-lipetsk/firebird that referenced this pull request Nov 17, 2023
It is additional change for patch FirebirdSQL#7845. Main issue is FirebirdSQL#7843. -------- This change adds the new feature of gbak.exe - generation UDF argument names during restore. This feature works only when database with ODS 13.0 (FB4) is restored. By default this feature is disabled. -------- New gbak command line switch: GEN_UDF_ARG_NAMES. New parameter for isc_action_svc_restore: isc_spb_res_gen_udf_arg_names=21 New burp-message with code 411 - "-GEN_UDF_ARG_NAMES generate udf argument names (ODS 13.0 and older)"
dmitry-lipetsk added a commit to dmitry-lipetsk/firebird that referenced this pull request Nov 22, 2023
Common issue is FirebirdSQL#7843 It is additional changes for PR FirebirdSQL#7845. ---- This patch allows gbak to correct data in RDB$ARGUMENT_NAME column of RDB$FUNCTION_ARGUMENTS table during restore. Two mode were implemented: for upgrade and for downgrade. By default, gbak does nothing but can check some data. The mode for disable any work and checks was added, too. ### Changes in GBAK.EXE To use a new functional is needed to define in command line new switch FIX_UDF_ARG_NAMES with one of additional parameters: DO_NOTHING, GEN_IF_NULL or SET_NULL - DO_NOTHING explicitly disables any manipulations and checks. - GEN_IF_NULL generates a new name of UDF input-argument if it is NULL. This is upgrade mode and allowed only for target ODS>=13 (FB4+). When gbak generates new name, it writes about it into log. For example: >gbak:restoring function UDF_DUMMY2_RPN_1 >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG1 for position 2 is generated >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG2 for position 3 is generated >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG3 for position 4 is generated - SET_NULL sets NULL into UDF input-argument. This is downgrade mode and allowed only for target ODS 12.x (FB3). When gbak resets name, it write about it into log. For example: >gbak:restoring function UDF_DUMMY2_RPN_1 >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG1 for position 2 is set to NULL >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG2 for position 3 is set to NULL >gbak: restoring argument for function UDF_DUMMY2_RPN_1 >gbak: argument name ARG3 for position 4 is set to NULL Note that gbak.exe, by fact, does not support the work with FB2.5 and earlier but can work with GBK-files those created with ones. When FIX_UDF_ARG_NAMES is used with incorrect parameter, gbak stops with the error. For example: - gbak: ERROR:FIX_UDF_ARG_NAMES mode GEN_IF_NULL is not supported for target ODS with runtime id 120 - gbak: ERROR:FIX_UDF_ARG_NAMES mode SET_NULL is not supported for target ODS with runtime id 130 ### Changes in GBAK.Service - New parameter for isc_action_svc_restore: isc_spb_res_fix_udf_arg_names=21 - The mode is passed as string: "DO_NOTHING", "GEN_IF_NULL" or "SET_NULL" Usage of isc_spb_res_fix_udf_arg_names has not been tested yet. ### Changes in source code - New internal gbak-error messages with codes 411-420 - GBAK-error messages with codes 406-410 are marked as used in FB5 ### Tests Updated gbak.exe was tested with: - backups of FB2, FB2.1, FB3 and FB4.0.4 - FB3
dmitry-lipetsk changed the title Contributor Author
| Hello, I added the support of theses things in my code and made tests for them. It is an archive with test logs and test database (without UDF dlls) -https://dropmefiles.com/dgfM4 (this link is valid within 2 weeks) These tests check the following things:
I tested all the standard types plus CSTRING. All is OK. There are only one problem with UDF_DUMMY2_BSA__BLOB (see #7877). I wait when you approve the PR #7870. After that I will merge with this PR the update of GBAK These changes in GBAK are not necessary to me but they allows to upgrade/downgrade exist databases and make this PR more clear. I can apply changes in GBAK later (in separate PR) to reduce the number of changes. Thanks. |
The check of udfReturnPos was moved from CreateAlterFunctionNode::executeAlter in CreateAlterFunctionNode::dsqlPass.
Contributor Author
| Hello, https://dropmefiles.com/yYJqu (this link is valid within 2 weeks) This archive contains results of updated gbak tests
All works OK. |
Contributor Author
| An additional idea about UDF-argument naming. It might make sense to use the "RDB$ARG" format for it. |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Hello.
It is the implementation of the second solution to issue #7843.
These changes force a server to do two things:
UDF
Metadata
It allows client to read the information about array element (type id, subtype, scale, charset id) and create a correct array.
If this patch is OK, I will create more complex tests for it on my side before final merging.
Thanks.