Skip to content

Extract external routines in one pass #8054

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Extract external routines in one pass #8054

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6425d68
Create external procedures and functions in one pass when extracting …
vasiliy-yashkov Mar 19, 2024
31b24ec
Remove external procedure type definition
vasiliy-yashkov Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dsql/DdlNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2941,7 +2941,7 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
if (secondPass)
{
P.RDB$PROCEDURE_TYPE.NULL = FALSE;
P.RDB$PROCEDURE_TYPE = (USHORT) prc_selectable;
P.RDB$PROCEDURE_TYPE = returns.getCount() != 0 ? (USHORT) prc_selectable : (USHORT) prc_executable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that procedure without output parameters cannot be selectable?
select count(*) from anyUDR()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select count(*) from anyUDR()

This causes SQL error code = -84; procedure TEST_UDR does not return any values. Even if the procedure has prc_selectable type.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves a completely different problem. #7699
As for creating external procedures and functions in one-pass, I agree with @asfernandes. This is not necessary and can even be harmful in some cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sim1984 yes, this is exactly the bug we were fixing. I suggested a single pass extraction as an extension to the original patch.

}
else
{
Expand Down
223 changes: 183 additions & 40 deletions src/isql/extract.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ static void list_package_headers();
static void list_procedure_bodies(SSHORT default_char_set_id);
static void list_procedure_headers(SSHORT default_char_set_id);
static void list_views();
static void list_external_procedures(SSHORT default_char_set_id);
static void list_external_functions(SSHORT default_char_set_id);

static const char* const Procterm = "^"; // TXNN: script use only

Expand Down Expand Up @@ -179,6 +181,8 @@ int EXTRACT_ddl(LegacyTables flag, const SCHAR* tabname)
list_domains(default_char_set_id);
list_all_tables(flag, default_char_set_id);
list_functions_legacy();
list_external_functions(default_char_set_id);
list_external_procedures(default_char_set_id);
list_functions_ods12_headers(default_char_set_id);
list_procedure_headers(default_char_set_id);
list_package_headers();
Expand Down Expand Up @@ -1509,6 +1513,8 @@ static void list_procedure_headers(SSHORT default_char_set_id)
FOR PRC IN RDB$PROCEDURES
WITH (PRC.RDB$SYSTEM_FLAG NE 1 OR PRC.RDB$SYSTEM_FLAG MISSING)
AND PRC.RDB$PACKAGE_NAME MISSING
AND PRC.RDB$ENGINE_NAME MISSING
AND PRC.RDB$ENTRYPOINT MISSING
SORTED BY PRC.RDB$PROCEDURE_NAME
if (header)
{
Expand Down Expand Up @@ -1573,6 +1579,8 @@ static void list_procedure_bodies(SSHORT default_char_set_id)
FOR PRC IN RDB$PROCEDURES
WITH (PRC.RDB$SYSTEM_FLAG NE 1 OR PRC.RDB$SYSTEM_FLAG MISSING)
AND PRC.RDB$PACKAGE_NAME MISSING
AND PRC.RDB$ENGINE_NAME MISSING
AND PRC.RDB$ENTRYPOINT MISSING
SORTED BY PRC.RDB$PROCEDURE_NAME
if (header)
{
Expand All @@ -1593,32 +1601,13 @@ static void list_procedure_bodies(SSHORT default_char_set_id)

// Print the procedure body

if (!PRC.RDB$ENTRYPOINT.NULL)
{
fb_utils::exact_name(PRC.RDB$ENTRYPOINT);
IUTILS_copy_SQL_id(PRC.RDB$ENTRYPOINT, SQL_identifier2, SINGLE_QUOTE);
isqlGlob.printf("EXTERNAL NAME %s%s", SQL_identifier2, NEWLINE);
}

if (!PRC.RDB$SQL_SECURITY.NULL)
{
const char* ss = PRC.RDB$SQL_SECURITY ? "SQL SECURITY DEFINER" : "SQL SECURITY INVOKER";
isqlGlob.printf("%s%s", ss, NEWLINE);
}

if (!PRC.RDB$ENGINE_NAME.NULL)
{
fb_utils::exact_name(PRC.RDB$ENGINE_NAME);
isqlGlob.printf("ENGINE %s", PRC.RDB$ENGINE_NAME);

if (!PRC.RDB$PROCEDURE_SOURCE.NULL)
{
isqlGlob.printf("%sAS '", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &PRC.RDB$PROCEDURE_SOURCE, true);
isqlGlob.printf("'%s", NEWLINE);
}
}
else if (!PRC.RDB$PROCEDURE_SOURCE.NULL)
if (!PRC.RDB$PROCEDURE_SOURCE.NULL)
{
isqlGlob.printf("AS %s", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &PRC.RDB$PROCEDURE_SOURCE);
Expand Down Expand Up @@ -3109,6 +3098,8 @@ static void list_functions_ods12_headers(SSHORT default_char_set_id)
WITH (FUN.RDB$SYSTEM_FLAG NE 1 OR FUN.RDB$SYSTEM_FLAG MISSING)
AND FUN.RDB$PACKAGE_NAME MISSING
AND FUN.RDB$MODULE_NAME MISSING
AND FUN.RDB$ENGINE_NAME MISSING
AND FUN.RDB$ENTRYPOINT MISSING
SORTED BY FUN.RDB$FUNCTION_NAME

if (header)
Expand Down Expand Up @@ -3161,6 +3152,8 @@ static void list_functions_ods12_bodies(SSHORT default_char_set_id)
WITH (FUN.RDB$SYSTEM_FLAG NE 1 OR FUN.RDB$SYSTEM_FLAG MISSING)
AND FUN.RDB$PACKAGE_NAME MISSING
AND FUN.RDB$MODULE_NAME MISSING
AND FUN.RDB$ENGINE_NAME MISSING
AND FUN.RDB$ENTRYPOINT MISSING
SORTED BY FUN.RDB$FUNCTION_NAME
if (header)
{
Expand All @@ -3184,32 +3177,13 @@ static void list_functions_ods12_bodies(SSHORT default_char_set_id)

// Print the function body

if (!FUN.RDB$ENTRYPOINT.NULL)
{
fb_utils::exact_name(FUN.RDB$ENTRYPOINT);
IUTILS_copy_SQL_id(FUN.RDB$ENTRYPOINT, SQL_identifier2, SINGLE_QUOTE);
isqlGlob.printf("EXTERNAL NAME %s%s", SQL_identifier2, NEWLINE);
}

if (!FUN.RDB$SQL_SECURITY.NULL)
{
const char* ss = FUN.RDB$SQL_SECURITY ? "SQL SECURITY DEFINER" : "SQL SECURITY INVOKER";
isqlGlob.printf("%s%s", ss, NEWLINE);
}

if (!FUN.RDB$ENGINE_NAME.NULL)
{
fb_utils::exact_name(FUN.RDB$ENGINE_NAME);
isqlGlob.printf("ENGINE %s", FUN.RDB$ENGINE_NAME);

if (!FUN.RDB$FUNCTION_SOURCE.NULL)
{
isqlGlob.printf("%sAS '", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &FUN.RDB$FUNCTION_SOURCE, true);
isqlGlob.printf("'%s", NEWLINE);
}
}
else if (!FUN.RDB$FUNCTION_SOURCE.NULL)
if (!FUN.RDB$FUNCTION_SOURCE.NULL)
{
isqlGlob.printf("AS %s", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &FUN.RDB$FUNCTION_SOURCE);
Expand Down Expand Up @@ -3594,3 +3568,172 @@ static void list_views()
return;
END_ERROR;
}

static void list_external_procedures(SSHORT default_char_set_id)
{
/**************************************
*
* l i s t _ e x t e r n a l _ p r o c e d u r e s
*
**************************************
*
* Functional description
* Create all external procedures declarations.
*
**************************************/

fb_assert(isqlGlob.major_ods >= ODS_VERSION12);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this is supposed to work with pre ODS13 databases ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current ISQL (FB 3-4-5) cannot extract procedures/functions from pre-ODS12 databases, so nothing really changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both list_external_procedures() and list_external_functions() is called unconditionally by EXTRACT_ddl(), see

https://github.com/FirebirdSQL/firebird/pull/8054/files#diff-580a2883156750bdebc94df7a24f090f8b288b49558292ae66fde9b21cbbc432R184

so, what happens in case of pre-ODS13 DB ?
Error will be raised ?
Nothing, as query below returns no rows ?
Why assert then ? It will not make DEBUG build happy, AFAIU.
I just want it to be clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error "field does not exist". You may find the same assert in list_procedure_bodies() which is also executed unconditionally. AFAIU, the only purpose of the assert is to warn that the extract function is not able to deal with older ODS. We may remove these asserts separately, if you disagree with their usage in general. They are not related to this PR and existed before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange logic, as for me - use assert and runtime error instead of condition and correct handling of old ODS.
I agree, it is not related with this PR. I never looked into this code before and thus wondering.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, it was discussed in fb-devel and we agreed to simplify the code this way, but it was ages ago...


bool header = true;
static const char* const create_procedure = "CREATE OR ALTER PROCEDURE %s ";

// Create the external procedures with their parameters
TEXT msg[MSG_LENGTH];

FOR PRC IN RDB$PROCEDURES
WITH (PRC.RDB$SYSTEM_FLAG NE 1 OR PRC.RDB$SYSTEM_FLAG MISSING)
AND PRC.RDB$PACKAGE_NAME MISSING
AND PRC.RDB$ENGINE_NAME NOT MISSING
AND PRC.RDB$ENTRYPOINT NOT MISSING
SORTED BY PRC.RDB$PROCEDURE_NAME
if (header)
{
isqlGlob.printf("%sCOMMIT WORK%s%s", NEWLINE, isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("SET AUTODDL OFF%s%s", isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("SET TERM %s %s%s", Procterm, isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("%s/* %s */%s", NEWLINE, "External procedures", NEWLINE);
header = false;
}
fb_utils::exact_name(PRC.RDB$PROCEDURE_NAME);
if (isqlGlob.db_SQL_dialect > SQL_DIALECT_V6_TRANSITION)
{
IUTILS_copy_SQL_id (PRC.RDB$PROCEDURE_NAME, SQL_identifier, DBL_QUOTE);
isqlGlob.printf(create_procedure, SQL_identifier);
}
else
{
isqlGlob.printf(create_procedure, PRC.RDB$PROCEDURE_NAME);
}

get_procedure_args(PRC.RDB$PROCEDURE_NAME, default_char_set_id);

fb_utils::exact_name(PRC.RDB$ENTRYPOINT);
IUTILS_copy_SQL_id(PRC.RDB$ENTRYPOINT, SQL_identifier2, SINGLE_QUOTE);
isqlGlob.printf("EXTERNAL NAME %s%s", SQL_identifier2, NEWLINE);

if (!PRC.RDB$SQL_SECURITY.NULL)
{
const char* ss = PRC.RDB$SQL_SECURITY ? "SQL SECURITY DEFINER" : "SQL SECURITY INVOKER";
isqlGlob.printf("%s%s", ss, NEWLINE);
}

fb_utils::exact_name(PRC.RDB$ENGINE_NAME);
isqlGlob.printf("ENGINE %s", PRC.RDB$ENGINE_NAME);

if (!PRC.RDB$PROCEDURE_SOURCE.NULL)
{
isqlGlob.printf("%sAS '", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &PRC.RDB$PROCEDURE_SOURCE, true);
isqlGlob.printf("'%s", NEWLINE);
}

isqlGlob.printf(" %s%s", Procterm, NEWLINE);

END_FOR
ON_ERROR
IUTILS_msg_get(GEN_ERR, msg, SafeArg() << isc_sqlcode(fbStatus->getErrors()));
STDERROUT(msg); // Statement failed, SQLCODE = %d\n\n
ISQL_errmsg(fbStatus);
return;
END_ERROR;

// Only reset the terminators if there were procedures to print
if (!header)
print_proc_suffix(obj_procedure);
}

static void list_external_functions(SSHORT default_char_set_id)
{
/**************************************
*
* l i s t _ e x t e r n a l _ f u n c t i o n s
*
**************************************
*
* Functional description
* Create all external functions declarations.
*
**************************************/

fb_assert(isqlGlob.major_ods >= ODS_VERSION12);

bool header = true;
static const char* const create_function = "CREATE OR ALTER FUNCTION %s ";

// Create the external functions with their parameters
TEXT msg[MSG_LENGTH];

FOR FUN IN RDB$FUNCTIONS
WITH (FUN.RDB$SYSTEM_FLAG NE 1 OR FUN.RDB$SYSTEM_FLAG MISSING)
AND FUN.RDB$PACKAGE_NAME MISSING
AND FUN.RDB$MODULE_NAME MISSING
AND FUN.RDB$ENGINE_NAME NOT MISSING
AND FUN.RDB$ENTRYPOINT NOT MISSING
SORTED BY FUN.RDB$FUNCTION_NAME

if (header)
{
isqlGlob.printf("%sCOMMIT WORK%s%s", NEWLINE, isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("SET AUTODDL OFF%s%s", isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("SET TERM %s %s%s", Procterm, isqlGlob.global_Term, NEWLINE);
isqlGlob.printf("%s/* %s */%s", NEWLINE, "External functions", NEWLINE);
header = false;
}

fb_utils::exact_name(FUN.RDB$FUNCTION_NAME);
if (isqlGlob.db_SQL_dialect > SQL_DIALECT_V6_TRANSITION)
{
IUTILS_copy_SQL_id(FUN.RDB$FUNCTION_NAME, SQL_identifier, DBL_QUOTE);
isqlGlob.printf(create_function, SQL_identifier);
}
else
{
isqlGlob.printf(create_function, FUN.RDB$FUNCTION_NAME);
}

get_function_args_ods12(FUN.RDB$FUNCTION_NAME, FUN.RDB$RETURN_ARGUMENT, default_char_set_id);

fb_utils::exact_name(FUN.RDB$ENTRYPOINT);
IUTILS_copy_SQL_id(FUN.RDB$ENTRYPOINT, SQL_identifier2, SINGLE_QUOTE);
isqlGlob.printf("EXTERNAL NAME %s%s", SQL_identifier2, NEWLINE);

if (!FUN.RDB$SQL_SECURITY.NULL)
{
const char* ss = FUN.RDB$SQL_SECURITY ? "SQL SECURITY DEFINER" : "SQL SECURITY INVOKER";
isqlGlob.printf("%s%s", ss, NEWLINE);
}

fb_utils::exact_name(FUN.RDB$ENGINE_NAME);
isqlGlob.printf("ENGINE %s", FUN.RDB$ENGINE_NAME);

if (!FUN.RDB$FUNCTION_SOURCE.NULL)
{
isqlGlob.printf("%sAS '", NEWLINE);
SHOW_print_metadata_text_blob(isqlGlob.Out, &FUN.RDB$FUNCTION_SOURCE, true);
isqlGlob.printf("'%s", NEWLINE);
}

isqlGlob.printf(" %s%s", Procterm, NEWLINE);

END_FOR
ON_ERROR
IUTILS_msg_get(GEN_ERR, msg, SafeArg() << isc_sqlcode(fbStatus->getErrors()));
STDERROUT(msg); // Statement failed, SQLCODE = %d\n\n
ISQL_errmsg(fbStatus);
return;
END_ERROR;

// Only reset the terminators if there were functions to print
if (!header)
print_proc_suffix(obj_udf);
}
Loading