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-25719] Fix an issue where [user_schema] was incorrectly stored or missing in the condition and action_definition columns of the db_trigger catalog during trigger creation or loaddb execution with legacy unload files #5729

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

jongmin-won
Copy link
Contributor

@jongmin-won jongmin-won commented Dec 19, 2024

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

Purpose

  1. CREATE TRIGGER 실행 시, SP가 포함된 경우 db_trigger 카탈로그의 action 및 condition 컬럼에 [user_schema]가 추가되지 않는 문제를 수정합니다.
  2. 과거의 unload 파일을 loaddb --no-user-specified-name 유틸리티를 사용해 트리거를 생성할 때, 실제 소유자를 찾아 db_trigger 카탈로그에 저장하도록 수정합니다.

Implementation

CREATE TRIGGER 동작 순서 수정

  1. Trigger 구문
CREATE TRIGGER [example]
BEFORE UPDATE ON [history](score)
EXECUTE INSERT INTO [update_logs] VALUES ([obj].[event_code], [obj].[score], '2024-12-11');
  1. Trigger 파싱 이후, Query Rewrite 수행 결과
insert into [dba.update_logs] values ([obj].[event_code], [obj].[score], '2024-12-11')
  1. Action(PT_SCOPE) 파싱 결과
  • 수정 전: 아래 SCOPE 구문이 에러 없이 정상적으로 컴파일된 경우, 2번에서 Rewrite된 구문을 카탈로그에 저장함.
SCOPE___ insert into [dba.update_logs] values ([obj].[event_code], [obj].[score], '2024-12-11') FROM ON  [public.history] obj, [public.history] new
  1. Action(PT_SCOPE)파싱 이후, Query Rewrite 수행 결과 (추가된 부분)
  • 수정 후: Action(PT_SCOPE) 파싱 결과를 다시 Rewrite하여 나온 구문을 카탈로그에 저장함.
insert into [dba.update_logs] ([event_code], [score], [dt]) values ([obj].[event_code], [obj].[score], '2024-12-11')

Remarks

N/A

jongmin-won added 3 commits December 19, 2024 21:08
…ondition columns of the db_trigger catalog when SP is included when performing create trigger, 2. When creating a trigger with the loaddb --no-user-specified-name utility, find the actual owner and modify it to save it in the db_trigger catalog, 3. When performing the unload utility, remove the owner’s [user_schema] from the action and condition columns.
@jongmin-won jongmin-won self-assigned this Dec 20, 2024
jongmin-won added 5 commits December 20, 2024 13:22
… rewritten into an unexecutable rewrite query.
…rewritten into an unexecutable rewrite query.
… rewritten into an unexecutable rewrite query.
…rewritten into an unexecutable rewrite query.
@jongmin-won jongmin-won changed the title [CBRD-25719] When executing loaddb with a past unload file (no user_schema) containing a trigger, the loaddb operation succeeds, but the trigger execution fails. [CBRD-25719] Fix an issue where [user_schema] was incorrectly stored or missing in the condition and action_definition columns of the db_trigger catalog during trigger creation or loaddb execution with legacy unload files Dec 20, 2024
@jongmin-won jongmin-won marked this pull request as ready for review December 23, 2024 04:11
jongmin-won added 3 commits December 24, 2024 00:57
…ory management issue in the change_trigger_action_query function might be the cause and have made adjustments to address it.
Comment on lines 1080 to 1094
char *p = NULL;
const char *remove_eval_prefix = "evaluate (";
size_t remove_eval_suffix_len;

p = strstr (result, remove_eval_prefix);
if (p != NULL)
{
p = (char *) memmove (p, p + strlen (remove_eval_prefix), strlen (p) - strlen (remove_eval_prefix) + 1);
}

remove_eval_suffix_len = strlen (p);
if (remove_eval_suffix_len > 0 && p[remove_eval_suffix_len - 1] == ')')
{
p[remove_eval_suffix_len - 1] = '\0';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

PR #5731 에서 다루는 코드와 동일한 것 같은데 함수로 만들어서 동일하게 유지 하는 것이 좋을 듯 합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코드 리뷰 감사합니다.
관련 부분은 PR #5731 에서 remove_appended_trigger_evaluate () 함수를 추가 했습니다.

Comment on lines 1265 to 1267
free_and_init (*new_trigger_stmt);
*new_trigger_stmt = strdup (new_trigger_stmt_str);
free_and_init (new_trigger_stmt_str);
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 위쪽에서 단 코멘트 중에 "return result; 로 충분할 듯 합니다." 이러 내용이 있었는데요,
그 코드를 수정하지 않는다면 반대로 이 부분에서 단지 아래와 같은 한 줄만 사용하면 될 것 같습니다.

*new_trigger_stmt = new_trigger_stmt_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.

코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.

@@ -11663,6 +11663,12 @@ pt_print_expr (PARSER_CONTEXT * parser, PT_NODE * p)
}
else
{
if (parser->flag.is_parsing_trigger == 1 && p->info.expr.flag != 0)
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
if (parser->flag.is_parsing_trigger == 1 && p->info.expr.flag != 0)
if (parser->flag.is_parsing_trigger && p->info.expr.flag)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.

@@ -13020,7 +13026,7 @@ pt_print_insert (PARSER_CONTEXT * parser, PT_NODE * p)

// TODO: [PL/CSQL] need refactoring
unsigned int save_custom = parser->custom_print;
if (parser->flag.is_parsing_static_sql == 1)
if (parser->flag.is_parsing_static_sql == 1 || parser->flag.is_parsing_trigger == 1)
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
if (parser->flag.is_parsing_static_sql == 1 || parser->flag.is_parsing_trigger == 1)
if (parser->flag.is_parsing_static_sql || parser->flag.is_parsing_trigger)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.

Comment on lines 1084 to 1088
p = strstr (result, remove_eval_prefix);
if (p != NULL)
{
p = (char *) memmove (p, p + strlen (remove_eval_prefix), strlen (p) - strlen (remove_eval_prefix) + 1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

p == 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.

코드 리뷰 감사합니다.
PR #5731에서 remove_appended_trigger_evaluate() 함수를 추가하고, 해당 함수 내부에 반영하여 수정했습니다.

if (with_evaluate)
{
char *p = NULL;
const char *remove_eval_prefix = "evaluate (";
Copy link
Contributor

@kangmin5505 kangmin5505 Dec 27, 2024

Choose a reason for hiding this comment

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

"evaluate (", "EVALUATE (", " ) "는
https://github.com/CUBRID/cubrid/pull/5731/files#r1896263225 에서 말씀드린 것과 마찬가지로, 한 곳에서 관리하는 것이 좋을 거 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코드 리뷰 감사합니다.
기존 EVALUATE 문자를 소문자로 작성한 이유는, 해당 문자열이 rewritten으로 작성되며 소문자로 변경되었고, evaluate를 제거할 때만 사용되기 때문입니다.
따라서, 해당 문자열은 PR #5731에서 작성한 remove_appended_trigger_evaluate() 함수 내부에서만 사용되도록 수정했습니다.

@jongmin-won jongmin-won added this to the fig-cake milestone Dec 30, 2024
@jongmin-won
Copy link
Contributor Author

Trigger Unload 시 [user_schema] 제거 (#5731) PR에서 중복된 코드가 있어, PR(#5731) 을 먼저 머지 이후 코드 리뷰 내용을 반영하여 수정하겠습니다.

@jongmin-won
Copy link
Contributor Author

jongmin-won commented Dec 31, 2024

github-checks / memory-monitor-check (pull_request) 에서 src/parser/compile.c: FAIL 된 이유

  • .github/workflows/check.yml 파일의 memory-monitor-check 부분에서 compile.c 파일이 grep 되어 FAIL 된 것으로 확인 됩니다.
filename=$(basename $f)
check_server_file=$(grep -F $filename cubrid/CMakeLists.txt)
if [ "$check_server_file" == "" ]; then
  result_for_f="PASS"
fi

예제

  • 아래 예시와 같이 compile.c 패턴이 grep에 출력 됨을 확인 했습니다.
$ filename=$(basename compile.c)
$ echo $filename
compile.c
$ grep $filename cubrid/CMakeLists.txt
  ${SP_DIR}/pl_struct_**compile.c**pp
  ${XASL_DIR}/**compile_c**ontext.h
$ grep -F $filename cubrid/CMakeLists.txt 
  ${SP_DIR}/pl_struct_**compile.c**pp

해결방안

개발 2팀 희수님이 담당으로 알고있어 관련 내용을 전달했습니다.

@jongmin-won
Copy link
Contributor Author

다음 PR (CUBRID/cubrid-testcases#2019) 에서 test_sql 과 test_plcsql 의 TC 답안지를 수정 합니다.

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

Successfully merging this pull request may close these issues.

4 participants