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-25352] Add a user schema to your stored procedures for consistency with other objects #5258

Open
wants to merge 25 commits into
base: feature/plcsql-p1n
Choose a base branch
from

Conversation

jongmin-won
Copy link
Contributor

@jongmin-won jongmin-won commented Jun 14, 2024

http://jira.cubrid.org/browse/CBRD-25352

Purpose
We will implement a feature to create a User Schema in Java Stored Procedure to ensure consistency with other objects such as tables, triggers, serial, servers, and synonyms.

Implementation
TO-DO

Remarks
AS-IS

  • Procedures and functions can be used by other users, independent of the owner.

CALL login ('test_user1', '') ON CLASS db_user;
CREATE FUNCTION function_hello() RETURN STRING AS LANGUAGE JAVA NAME 'SpCubrid.HelloCubrid() return java.lang.String';

CALL login ('dba', '') ON CLASS db_user;
SELECT function_hello() FROM db_root;

TO-BE

  • When a user other than the owner uses a procedure or function, they must specify the owner_name.

CALL login ('test_user1', '') ON CLASS db_user;
CREATE FUNCTION function_hello() RETURN STRING AS LANGUAGE JAVA NAME 'SpCubrid.HelloCubrid() return java.lang.String';

CALL login ('dba', '') ON CLASS db_user;
SELECT test_user1.function_hello() FROM db_root;

… feature/plcsql-p1n branch to add a user schema to a stored procedure.
@jongmin-won jongmin-won self-assigned this Jun 14, 2024
jongmin-won added 8 commits June 18, 2024 09:04
…ditionally, class methods are system tables and do not require user_schema. ex) 'add_user', 'drop_user', 'find_user'
…Additionally, Resolves an error where the CALL PROCEDURE/FUNCTION statement does not work correctly.
…ocedure object, 2) select hello(); Modified so that [user_name.]hello is displayed when an error message occurs because the corresponding sp cannot be found during execution.
…ion user when executing a CREATE OR REPLACE PROCEDURE/FUNCTION, 2) Add user_schema to SP when performing a schema unload
…d within an if-else-statement when the CALL statement is executed. 2) Added a part to remove the owner and user_schema when the unloaddb utility is executed as a general user, provided that the owner and user_schema are the same.
@jongmin-won jongmin-won marked this pull request as ready for review July 5, 2024 02:10
jongmin-won added 8 commits July 9, 2024 22:10
…stem method 와 user method (Function& Stored_procedure) 구분을 grammar.y에서 확인하도록 수정
…st에 세션 변수가 있는 경우, csql_grammar.y에서는 세션 변수에 클래스, 객체 또는 상수 중 어떤 값이 포함되어 있는지 알 수 없습니다. 따라서, pt_bind_names()에서 java_stored_procedure가 아니고 on_call_target 값이 있으면 method로 간주하여 [user_schema]를 제거합니다.
…y value 로 잘 못 수정되는 부분 수정, 2) PT_FUNCTION 노드를 수행시 generic_name 값을 항상 downcase 하도록 수정
…evelop과 같이 data_type 토큰을 sp_param_type 토큰으로 변경했습니다.
@@ -4092,10 +4098,10 @@ alter_stmt
DBG_PRINT}}
| ALTER /* 1 */
procedure_or_function /* 2 */
identifier /* 3 */
procedure_name /* 3 */
Copy link
Contributor

Choose a reason for hiding this comment

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

이 룰에 따르면 procedure_namefunction_name을 따로 정의하면 모호하게 됩니다.
procedure_or_function_name 이라고 공통된 이름을 만드는 것이 어떤가요?
이렇게 수정을 한다면 동일하게 procedure_name_listfunction_name_list도 하나로 통합해야 할 것입니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 코드 리뷰 감사합니다.
제 생각에도 불필요한 토큰을 사용하고 있는 것 같아서 말씀하신 것처럼 procedure_name과 function_name을 procedure_or_function_name처럼 하나로 통합했습니다.

…cedure_or_function_name token with a common name. 2) Fixed the logic when PT_DOT_ node type is changed to PT_METHOD_CALL node type when STORED_PROCEDURE is executed.
procedure_or_function_name_list
: procedure_or_function_name_list ',' procedure_or_function_name
{{

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DBG_TRACE_GRAMMAR(procedure_or_function_name_list, : procedure_or_function_name_list ',' procedure_or_function_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.

Debugging을 위한 정보를 추가했습니다.

DBG_PRINT}}
| procedure_or_function_name
{{

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DBG_TRACE_GRAMMAR(procedure_or_function_name_list,| procedure_or_function_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.

Debugging을 위한 정보를 추가했습니다.

@@ -12872,7 +12908,7 @@ sp_param_list
sp_param_def
: identifier
opt_sp_in_out
data_type
sp_param_type
Copy link
Contributor

Choose a reason for hiding this comment

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

data_type에서 sp_param_type으로 변경하면 CURSOR 같은 것도 포함되는데 의도하는 내용이 맞나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 의도한 내용이 맞습니다.

이미 Develop에 적용한 내용인데, Merge 과정에서 누락된 부분이 있어 수정했습니다.

sprintf (new_name_str, "%s.%s", downcase_owner_name, name_str);

/* change the unique_name */
if (new_name_str != NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

new_name_str은 정적인 배열입니다. 이 체크는 의미가 없습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 코드 리뷰를 주셔서 감사합니다.
정적인 배열을 체크하는 부분을 제거했습니다.

sm_downcase_name (owner_str, downcase_owner_name, DB_MAX_USER_LENGTH);
sprintf (new_name_str, "%s.%s", downcase_owner_name, sm_remove_qualifier_name (name_str));

if (new_name_str != NULL)
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.

좋은 코드 리뷰를 주셔서 감사합니다.
정적인 배열을 체크하는 부분을 제거했습니다.

* If (dot.arg1->node_type == PT_NAME) & (dot.arg2->node_type == PT_FUNCTION), pt_bind_name_or_path_in_scope() always returns NULL and has an er_errid() value.
* Therefore, er_errid() that occurs in pt_bind_name_or_path_in_scope must be reset.
*/
if (er_errid () == NO_ERROR)
Copy link
Contributor

@hyunikn hyunikn Jul 22, 2024

Choose a reason for hiding this comment

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

혹시 != 를 == 로 잘못 쓰신 것 인가요.

{
/* In system class names, owner name can be NULL. Otherwise, owner name must not be NULL. */
assert (au_is_dba_group_member (Au_user));
assert (sm_check_system_class_by_name (PT_NAME_ORIGINAL (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 이름이 시스템 class/vclass 이름이라고 assert 하고 있는데, 맞는 내용인가요.

@@ -540,6 +540,12 @@ struct json_t;
|| (n->node_type == PT_UPDATE && n->info.update.spec->info.spec.remote_server_name) \
|| (n->node_type == PT_MERGE && n->info.merge.into->info.spec.remote_server_name))

#define PT_CHECK_USER_SCHEMA_PROCEDURE_OR_FUNCTION(n) \
Copy link
Contributor

Choose a reason for hiding this comment

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

이 매크로가 검사하는 내용에 비해 매크로의 이름이 잘 이해가 되지 않습니다.
user schema procedure or function 임을 검사하는 것이 generic_name 에 dot 이 없다는 것을 검사하는 것인가요.

@@ -3245,6 +3245,19 @@ pt_bind_names (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue
node->info.method_call.on_call_target = node->info.method_call.arg_list;
node->info.method_call.arg_list = node->info.method_call.arg_list->next;
node->info.method_call.on_call_target->next = NULL;

Copy link
Contributor

Choose a reason for hiding this comment

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

회의 때 on 을 명시한 문법만 남겨 두기로 했던 것으로 기억합니다.
그렇다면, 이 if block 은 없어지게 되나요?

@@ -65,6 +65,7 @@ static int sp_builtin_init ()
a.is_system_generated = true;

// DBMS_OUTPUT.enable
v.unique_name = "dbms_output.enable";
Copy link
Contributor

Choose a reason for hiding this comment

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

dbms_output 이라는 user 가 생긴 것인가요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants