From 2e5dd6a8c09fcc0938e3ba32fe2d861b45c3400b Mon Sep 17 00:00:00 2001 From: "HOME5\\dmitr" Date: Tue, 14 Nov 2023 20:29:48 +0300 Subject: [PATCH 1/3] Generation of UDF argument names It is the implementation of the second solution to issue #7843. 1. Server generates UDF argument names and saves them in RDB$ARGUMENT_NAME column. Structure of name: ARG. 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 --- src/dsql/DdlNodes.epp | 56 ++++++++++++++++++++++++++++++++++++------ src/dsql/ExprNodes.cpp | 49 ++++++++++++++++++++++++++++++------ src/dsql/StmtNodes.cpp | 2 +- src/dsql/dsql.h | 14 ++++++++++- src/dsql/metd.epp | 4 +-- 5 files changed, 106 insertions(+), 19 deletions(-) diff --git a/src/dsql/DdlNodes.epp b/src/dsql/DdlNodes.epp index 8652cb7b7b7..9f9c972dc45 100644 --- a/src/dsql/DdlNodes.epp +++ b/src/dsql/DdlNodes.epp @@ -1635,19 +1635,61 @@ DdlNode* CreateAlterFunctionNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) StrArray names; - for (FB_SIZE_T i = 0; i < parameters.getCount(); ++i) + if (this->isUdf()) { - const ParameterClause* const parameter = parameters[i]; + // Generation of UDF input argument names. + // They will be used for work with SCALAR_ARRAY-arguments. - if (names.exist(parameter->name.c_str())) + unsigned argNum = 0; + + for (FB_SIZE_T i = 0; i < parameters.getCount(); ++i) { - status_exception::raise( - Arg::Gds(isc_sqlerr) << Arg::Num(-901) << - Arg::Gds(isc_dsql_duplicate_spec) << Arg::Str(parameter->name)); + ParameterClause* const parameter = parameters[i]; + + fb_assert(!parameter->name.hasData()); + + if (this->returnType == NULL && this->udfReturnPos == (i + 1)) + { + // It is a parameter that is marked as RETURN + + fb_assert(!names.exist(parameter->name.c_str())); + continue; + } + + // It is an input argument + ++argNum; + parameter->name.printf("ARG%d", argNum); + fb_assert(parameter->name.hasData()); + + if (names.exist(parameter->name.c_str())) + { + status_exception::raise( + Arg::Gds(isc_sqlerr) << Arg::Num(-901) << + Arg::Gds(isc_dsql_duplicate_spec) << Arg::Str(parameter->name)); + } + + names.add(parameter->name.c_str()); } + } + else + { + fb_assert(!this->isUdf()); + + for (FB_SIZE_T i = 0; i < parameters.getCount(); ++i) + { + const ParameterClause* const parameter = parameters[i]; + + fb_assert(parameter->name.hasData()); + + if (names.exist(parameter->name.c_str())) + { + status_exception::raise( + Arg::Gds(isc_sqlerr) << Arg::Num(-901) << + Arg::Gds(isc_dsql_duplicate_spec) << Arg::Str(parameter->name)); + } - if (parameter->name.hasData()) // legacy UDFs has unnamed parameters names.add(parameter->name.c_str()); + } } PASS1_check_unique_fields_names(names, localDeclList); diff --git a/src/dsql/ExprNodes.cpp b/src/dsql/ExprNodes.cpp index 7a7f76bffb5..0bd60a31283 100644 --- a/src/dsql/ExprNodes.cpp +++ b/src/dsql/ExprNodes.cpp @@ -13140,21 +13140,54 @@ ValueExprNode* UdfCallNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) unsigned pos = 0; - for (auto& arg : node->args->items) + for (auto p_nodeArg = node->args->items.begin(), e_nodeArg = node->args->items.end(); + p_nodeArg != e_nodeArg; + ++p_nodeArg, ++pos) { - if (pos < node->dsqlFunction->udf_arguments.getCount()) + if (node->dsqlFunction->udf_arguments.getCount() <= pos) { - PASS1_set_parameter_type(dsqlScratch, arg, - [&] (dsc* desc) { *desc = node->dsqlFunction->udf_arguments[pos]; }, - false); + // TODO: We should complain here in the future! The parameter is + // out of bounds or the function doesn't declare input params. + + break; + } + + auto& nodeArg = (*p_nodeArg); + + if (!nodeArg) + { + continue; + } + + const auto& udfArg = node->dsqlFunction->udf_arguments[pos]; + + PASS1_set_parameter_type(dsqlScratch, nodeArg, + [&] (dsc* desc) { *desc = udfArg.desc; }, + false); + + // We will provide a client with the additional information about + // the linked UDF argument of DSQL parameter with array. + + 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) + { + ParameterNode* const paramNode = nodeAs(nodeArg); + dsql_par* const parameter = paramNode->dsqlParameter; + parameter->par_rel_name = node->dsqlFunction->udf_name.identifier; + parameter->par_name = udfArg.name; - ++pos; + fb_assert(!parameter->par_rel_name.isEmpty()); + fb_assert(!parameter->par_name.isEmpty()); + } } return node; diff --git a/src/dsql/StmtNodes.cpp b/src/dsql/StmtNodes.cpp index 9a84674f527..86a47d588f2 100644 --- a/src/dsql/StmtNodes.cpp +++ b/src/dsql/StmtNodes.cpp @@ -1657,7 +1657,7 @@ DeclareSubFuncNode* DeclareSubFuncNode::dsqlPass(DsqlCompilerScratch* dsqlScratc if (!implemetingForward) { // ASF: dsqlFunction->udf_arguments is only checked for its count for now. - dsqlFunction->udf_arguments.add(dsc()); + dsqlFunction->udf_arguments.add(dsql_udf_arg(pool, dsc(), nullptr)); } if (param->defaultClause) diff --git a/src/dsql/dsql.h b/src/dsql/dsql.h index 737ce32246c..396f6b1b533 100644 --- a/src/dsql/dsql.h +++ b/src/dsql/dsql.h @@ -358,6 +358,18 @@ enum prc_flags_vals { PRC_subproc = 4 // Sub procedure }; +//! User defined function argument +struct dsql_udf_arg +{ + dsc desc; + MetaName name; + + dsql_udf_arg(MemoryPool& p, const dsc& d, const char* n) + :desc(d), name(p, n) + { + } +}; + //! User defined function block class dsql_udf : public pool_alloc { @@ -375,7 +387,7 @@ class dsql_udf : public pool_alloc //USHORT udf_character_length; USHORT udf_flags; QualifiedName udf_name; - Firebird::Array udf_arguments; + Firebird::Array udf_arguments; bool udf_private; // Packaged private function SSHORT udf_def_count; // number of inputs with default values }; diff --git a/src/dsql/metd.epp b/src/dsql/metd.epp index 60ff0a5f1c4..c6b50c8e2c7 100644 --- a/src/dsql/metd.epp +++ b/src/dsql/metd.epp @@ -790,7 +790,7 @@ dsql_udf* METD_get_function(jrd_tra* transaction, DsqlCompilerScratch* dsqlScrat defaults++; } - userFunc->udf_arguments.add(d); + userFunc->udf_arguments.add(dsql_udf_arg(dbb->dbb_pool, d, X.RDB$ARGUMENT_NAME)); } } END_FOR @@ -875,7 +875,7 @@ dsql_udf* METD_get_function(jrd_tra* transaction, DsqlCompilerScratch* dsqlScrat defaults++; } - userFunc->udf_arguments.add(d); + userFunc->udf_arguments.add(dsql_udf_arg(dbb->dbb_pool, d, X.RDB$ARGUMENT_NAME)); } } } From aff4e44f7d5120fee8953eb0343f05760aa7cff7 Mon Sep 17 00:00:00 2001 From: "HOME5\\dmitr" Date: Wed, 15 Nov 2023 11:32:07 +0300 Subject: [PATCH 2/3] Detection of RETURN-PARAMETER was optimized. 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. --- src/dsql/DdlNodes.epp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dsql/DdlNodes.epp b/src/dsql/DdlNodes.epp index 9f9c972dc45..7f4e1b710ee 100644 --- a/src/dsql/DdlNodes.epp +++ b/src/dsql/DdlNodes.epp @@ -1648,10 +1648,13 @@ DdlNode* CreateAlterFunctionNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) fb_assert(!parameter->name.hasData()); - if (this->returnType == NULL && this->udfReturnPos == (i + 1)) + if (this->udfReturnPos == (i + 1)) { // It is a parameter that is marked as RETURN + fb_assert(this->returnType == nullptr); // research. it is assumed. + fb_assert(this->udfReturnPos > 0); // paranoia + fb_assert(!names.exist(parameter->name.c_str())); continue; } From d6affaead69925a3c7cd7d0e239315bbdc41ca75 Mon Sep 17 00:00:00 2001 From: "HOME5\\dmitr" Date: Mon, 27 Nov 2023 00:24:48 +0300 Subject: [PATCH 3/3] The problem with udfReturnPos was fixed The check of udfReturnPos was moved from CreateAlterFunctionNode::executeAlter in CreateAlterFunctionNode::dsqlPass. --- src/dsql/DdlNodes.epp | 49 +++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/src/dsql/DdlNodes.epp b/src/dsql/DdlNodes.epp index 7f4e1b710ee..76c8762a4b8 100644 --- a/src/dsql/DdlNodes.epp +++ b/src/dsql/DdlNodes.epp @@ -1631,6 +1631,40 @@ DdlNode* CreateAlterFunctionNode::dsqlPass(DsqlCompilerScratch* dsqlScratch) { dsqlScratch->flags |= (DsqlCompilerScratch::FLAG_BLOCK | DsqlCompilerScratch::FLAG_FUNCTION); + if (this->isUdf()) + { + // check udfReturnPos + const dsql_fld* const field = this->returnType ? this->returnType->type : NULL; + + if (field) + { + // CVC: This is case of "returns [by value|reference]". + fb_assert(udfReturnPos == 0); + } + else + { + // CVC: This is case of "returns parameter " + + // see return_value in parser + fb_assert(udfReturnPos > 0); + + if (udfReturnPos < 1 || ULONG(udfReturnPos) > parameters.getCount()) + { + // CVC: We should devise new msg "position should be between 1 and #params"; + // here it is: dsql_udf_return_pos_err + + // External functions can not have more than 10 parameters + // Not strictly correct -- return position error + status_exception::raise( + Arg::Gds(isc_sqlerr) << Arg::Num(-607) << + Arg::Gds(isc_dsql_command_err) << // gds__extern_func_err + Arg::Gds(isc_dsql_udf_return_pos_err) << Arg::Num(parameters.getCount())); + } + } + + fb_assert(udfReturnPos >= 0 && ULONG(udfReturnPos) <= parameters.getCount()); + } + // check for duplicated parameters and declaration names StrArray names; @@ -1983,19 +2017,8 @@ bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch* else // CVC: This is case of "returns parameter " { // Function modifies an argument whose value is the function return value. - - if (udfReturnPos < 1 || ULONG(udfReturnPos) > parameters.getCount()) - { - // CVC: We should devise new msg "position should be between 1 and #params"; - // here it is: dsql_udf_return_pos_err - - // External functions can not have more than 10 parameters - // Not strictly correct -- return position error - status_exception::raise( - Arg::Gds(isc_sqlerr) << Arg::Num(-607) << - Arg::Gds(isc_dsql_command_err) << // gds__extern_func_err - Arg::Gds(isc_dsql_udf_return_pos_err) << Arg::Num(parameters.getCount())); - } + // It was checked in CreateAlterFunctionNode::dsqlPass + fb_assert(!(udfReturnPos < 1 || ULONG(udfReturnPos) > parameters.getCount())); // We'll verify that SCALAR_ARRAY can't be used as a return type. // The support for SCALAR_ARRAY is only for input parameters.