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

[CBRD-25707] With the addition of [user_schema] to SP, the --no-user-specified-name option of the loaddb utility has been fixed to function correctly, even when SPs are included in unload files from the past. #5659

Open
wants to merge 5 commits into
base: feature/plcsql-p1n
Choose a base branch
from
17 changes: 8 additions & 9 deletions src/parser/name_resolution.c
Original file line number Diff line number Diff line change
Expand Up @@ -3499,9 +3499,8 @@ pt_bind_names (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue
case PT_FUNCTION:
if (node->info.function.function_type == PT_GENERIC)
{
const char *dot = NULL;
const char *current_schema_name = NULL;
char buffer[SM_MAX_IDENTIFIER_LENGTH];
MOP sp_mop = NULL;
char sp_unique_name[SM_MAX_IDENTIFIER_LENGTH + 1];
node->info.function.function_type = pt_find_function_type (node->info.function.generic_name);

if (node->info.function.function_type == PT_GENERIC)
Expand All @@ -3522,14 +3521,14 @@ pt_bind_names (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue
* nodes PT_FUNCTION. If so, pt_make_stored_procedure() and pt_make_method_call() will
* translate it into a method_call.
*/
if (jsp_is_exist_stored_procedure (node->info.function.generic_name))
sp_mop = jsp_find_stored_procedure (node->info.function.generic_name, DB_AUTH_NONE);
if (sp_mop != NULL)
{
dot = strchr (node->info.function.generic_name, '.');
if (dot == NULL)
sp_unique_name[0] = '\0';
jsp_get_unique_name (sp_mop, sp_unique_name, DB_MAX_IDENTIFIER_LENGTH + 1);
if (sp_unique_name[0] != '\0')
{
current_schema_name = sc_current_schema_name ();
sprintf (buffer, "%s.%s", current_schema_name, node->info.function.generic_name);
node->info.function.generic_name = pt_append_string (parser, NULL, buffer);
node->info.function.generic_name = pt_append_string (parser, NULL, sp_unique_name);
}

node1 = pt_resolve_stored_procedure (parser, node, bind_arg);
Expand Down
92 changes: 92 additions & 0 deletions src/query/execute_statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -20250,6 +20250,98 @@ do_find_synonym_by_query (const char *name, char *buf, int buf_size)
#undef QUERY_BUF_SIZE
}

int
do_find_stored_procedure_by_query (const char *name, char *buf, int buf_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

쿼리 내용을 보니 이름은 갖고 소유자는 현재 스키마와 다른 경우를 찾는 것 같습니다.
함수 이름에 이런 사정이 잘 드러나지 않는 듯 합니다.
함수 이름으로 do_find_sp_of_another_owner 정도가 어떨까요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1. 기존에 동일한 기능을 수행하는 함수들과의 일관성을 유지하기 위해 do_find_stored_procedure_by_query() 함수의 이름은 변경하지 않았습니다.

  • 아래 함수들은 object_name을 전달 받아 각 카탈로그를 조회하고 unique_name을 찾는 기능을 합니다.
do_find_class_by_query
do_find_serial_by_query
do_find_trigger_by_query
do_find_synonym_by_query

2. 코드의 가독성을 높이기 위해 제안해 주신 함수 이름을 참고하여 jsp_find_sp_of_another_owner 함수를 추가하였으며, 해당 함수에서 다른 소유자의 SP를 찾을 수 있도록 수정했습니다.

  • TO-BE의 Call Stack과 같이 동작하도록 수정 했습니다.

AS-IS

jsp_find_stored_procedure
-> do_find_stored_procedure_by_query

TO-BE

jsp_find_stored_procedure
-> jsp_find_sp_of_another_owner
-> do_find_stored_procedure_by_query

{
#define QUERY_BUF_SIZE 2048
jongmin-won marked this conversation as resolved.
Show resolved Hide resolved
DB_QUERY_RESULT *query_result = NULL;
DB_QUERY_ERROR query_error;
DB_VALUE value;
const char *query = NULL;
char query_buf[QUERY_BUF_SIZE] = { '\0' };
const char *current_schema_name = NULL;
const char *sp_name = NULL;
int error = NO_ERROR;

db_make_null (&value);
query_error.err_lineno = 0;
query_error.err_posno = 0;

if (name == NULL || name[0] == '\0')
{
ERROR_SET_WARNING (error, ER_OBJ_INVALID_ARGUMENTS);
return error;
}

assert (buf != NULL);

current_schema_name = sc_current_schema_name ();

sp_name = sm_remove_qualifier_name (name);
Copy link
Contributor

Choose a reason for hiding this comment

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

sp_name이 NULL일 경우는 체크할 필요가 없을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sp_name이 NULL인 경우, WHERE 조건에서 [sp_name] = NULL로 조회하게 됩니다.
이로 인해 결과가 0건으로 반환되며, 20315번 라인에서 에러가 설정됩니다.

query = "SELECT [unique_name] FROM [%s] WHERE [sp_name] = '%s' AND [owner].[name] != UPPER ('%s')";
assert (QUERY_BUF_SIZE > snprintf (NULL, 0, query, CT_STORED_PROC_NAME, sp_name, current_schema_name));
snprintf (query_buf, QUERY_BUF_SIZE, query, CT_STORED_PROC_NAME, sp_name, current_schema_name);
assert (query_buf[0] != '\0');

error = db_compile_and_execute_local (query_buf, &query_result, &query_error);
if (error < NO_ERROR)
{
ASSERT_ERROR ();
goto end;
}

error = db_query_first_tuple (query_result);
if (error != DB_CURSOR_SUCCESS)
{
if (error == DB_CURSOR_END)
{
ERROR_SET_WARNING_1ARG (error, ER_SP_NOT_EXIST, name);
}
else
{
ASSERT_ERROR ();
}

goto end;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

다른 에러가 발생할 경우는 없나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네, DB_CURSOR_SUCCESS와 DB_CURSOR_END를 제외한 모든 경우는 ASSERT_ERROR()로 처리됩니다.
추가로 확인한 결과, db_query_first_tuple 내부에서 cursor_first_tuple 함수로 인해 발생할 수 있는 상태는 아래 세 가지입니다.

DB_CURSOR_ERROR
DB_CURSOR_END
DB_CURSOR_SUCCESS


error = db_query_get_tuple_value (query_result, 0, &value);
if (error != NO_ERROR)
{
ASSERT_ERROR ();
goto end;
}

if (!DB_IS_NULL (&value))
{
assert (strlen (db_get_string (&value)) < buf_size);
strcpy (buf, db_get_string (&value));
}
else
{
/* unique_name must not be null. */
ASSERT_ERROR_AND_SET (error);
goto end;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if (DB_IS_NULL) 로 하는 게 읽기 편할 것 같습니다.


error = db_query_next_tuple (query_result);
if (error != DB_CURSOR_END)
{
/* No result can be returned because unique_name is not unique. */
buf[0] = '\0';
}

end:
if (query_result)
{
db_query_end (query_result);
query_result = NULL;
}

return error;
#undef QUERY_BUF_SIZE
}

/*
* do_set_trace_to_query_flag() -
* return: void
Expand Down
1 change: 1 addition & 0 deletions src/query/execute_statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,4 +207,5 @@ extern int do_find_class_by_query (const char *name, char *buf, int buf_size);
extern int do_find_serial_by_query (const char *name, char *buf, int buf_size);
extern int do_find_trigger_by_query (const char *name, char *buf, int buf_size);
extern int do_find_synonym_by_query (const char *name, char *buf, int buf_size);
extern int do_find_stored_procedure_by_query (const char *name, char *buf, int buf_size);
#endif /* _EXECUTE_STATEMENT_H_ */
31 changes: 25 additions & 6 deletions src/sp/jsp_cl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
#include "oid.h"
#include "string_buffer.hpp"
#include "db_value_printer.hpp"
#include "execute_statement.h"

#define PT_NODE_SP_NAME(node) \
(((node)->info.sp.name == NULL) ? "" : \
Expand Down Expand Up @@ -154,6 +155,7 @@ jsp_find_stored_procedure (const char *name, DB_AUTH purpose)
DB_VALUE value;
int save, err = NO_ERROR;
char *checked_name;
char other_class_name[DB_MAX_IDENTIFIER_LENGTH];

if (!name)
{
Expand All @@ -169,8 +171,29 @@ jsp_find_stored_procedure (const char *name, DB_AUTH purpose)
if (er_errid () == ER_OBJ_OBJECT_NOT_FOUND)
{
er_clear ();
err = ER_SP_NOT_EXIST;
er_set (ER_WARNING_SEVERITY, ARG_FILE_LINE, err, 1, checked_name);

/* This is the case when the loaddb utility is executed with the --no-user-specified-name option as the dba user. */
if (db_get_client_type () == DB_CLIENT_TYPE_ADMIN_LOADDB_COMPAT)
{
other_class_name[0] = '\0';

err = do_find_stored_procedure_by_query (name, other_class_name, DB_MAX_IDENTIFIER_LENGTH);
jongmin-won marked this conversation as resolved.
Show resolved Hide resolved
if (other_class_name[0] != '\0')
{
db_make_string (&value, other_class_name);
mop = db_find_unique (db_find_class (SP_CLASS_NAME), SP_ATTR_UNIQUE_NAME, &value);
if (er_errid () == ER_OBJ_OBJECT_NOT_FOUND)
{
err = ER_SP_NOT_EXIST;
er_set (ER_WARNING_SEVERITY, ARG_FILE_LINE, err, 1, other_class_name);
}
}
}
else
{
err = ER_SP_NOT_EXIST;
er_set (ER_WARNING_SEVERITY, ARG_FILE_LINE, err, 1, checked_name);
}
Copy link
Member

Choose a reason for hiding this comment

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

클라이언트 타입이 일반적인 경우가 아닌 LOADDB에서만 동작하므로 별도의 함수로 분리하고, other_class_name이 해당 함수 호출 시에 생성되도록 하면 좋을 것 같습니다. 가독성 면에서도 주요 로직을 파악하기에 함수로 분리하는 것이 좋아보입니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1. 클라이언트 타입이 일반적인 경우가 아닌 LOADDB에서만 동작하므로 별도의 함수로 분리

loaddb에서만 동작되도록 별도의 함수로 분리하기가 어렵습니다.
분리가 어려운 이유

  • 현재 문제가 되는 부분은 CREATE [FUNCTION | PROCEDURE]이 아니라 ALTER VCLASS .. CHANGE QUERY입니다.
  • pt_validate_query_spec 함수에서 view 쿼리를 다시 파싱하면서 SP를 확인해야 하므로, jsp_find_stored_procedure 함수 내부에서 sp_name을 이용해 unique_name을 찾을 수 있도록 수정이 필요합니다.
loaddb_user
-> ldr_exec_query_from_file
...
-> pt_check_alter
-> pt_validate_query_spec
-> pt_check_vclass_query_spec

다른 함수 확인 결과

  • class 확인을 위한 db_find_class 함수 내부에서도 --no-user-specified-name 옵션으로 loaddb 유틸리티를 실행하는 경우, do_find_class_by_query 함수를 사용해 unique_name을 찾습니다.
    (실제 class의 object 를 loaddb 할 때, db_find_class 함수는 사용하지 않고, db_find_class_with_purpose 함수를 사용합니다.)
  • 이와 동일한 방식으로 SP에도 추가 작업을 했습니다.
db_find_class
-> db_find_class_with_purpose
-> sm_find_class_with_purpose
-> locator_find_class_with_purpose
/* --no-user-specified-name 옵션을 사용하여 loaddb 유틸리티를 실행하는 경우 */
-> do_find_class_by_query

2. 가독성을 위해 함수 분리

  • 현익님의 코드 리뷰 내용을 참고하여 jsp_find_sp_of_another_owner 함수를 추가하였으며, 해당 함수에서 다른 소유자의 SP를 찾을 수 있도록 수정했습니다.

AS-IS

jsp_find_stored_procedure
-> do_find_stored_procedure_by_query

TO-BE

jsp_find_stored_procedure
-> jsp_find_sp_of_another_owner
-> do_find_stored_procedure_by_query

}

if (mop)
Expand Down Expand Up @@ -1435,8 +1458,6 @@ jsp_check_stored_procedure_name (const char *str)
char tmp[SM_MAX_IDENTIFIER_LENGTH + 2];
char *name = NULL;
static int dbms_output_len = strlen ("dbms_output.");
char owner_name[DB_MAX_USER_LENGTH];
owner_name[0] = '\0';


if (strncasecmp (str, "dbms_output.", dbms_output_len) == 0)
Expand All @@ -1449,8 +1470,6 @@ jsp_check_stored_procedure_name (const char *str)
sm_user_specified_name (str, buffer, SM_MAX_IDENTIFIER_LENGTH);
}

// sm_downcase_name (str, buffer, SM_MAX_IDENTIFIER_LENGTH);

name = strdup (buffer);

return name;
Expand Down
Loading