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
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
09c9346
1. Fix the issue where [user_schema] is not added to the action and c…
Dec 19, 2024
4a81ce2
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Dec 20, 2024
55a8430
Remove [user_schema] from the condition column when the owner perform…
Dec 20, 2024
195ad20
Remove warning messages in the parse_tree_cl file.
Dec 20, 2024
5adef67
Fixed an issue where the action query entered by the user was rewritt…
Dec 20, 2024
fc4c1d8
Second. fixed an issue where the action query entered by the user was…
Dec 20, 2024
5c34b6a
Third. fixed an issue where the action query entered by the user was …
Dec 20, 2024
3cc99b9
Fourth. fixed an issue where the action query entered by the user was…
Dec 20, 2024
2d1f5b8
Fifth. fixed an issue where the action query entered by the user was …
Dec 20, 2024
1034d83
Sixth. fixed an issue where the action query entered by the user was …
Dec 20, 2024
897bb06
Seventh. fixed an issue where the action query entered by the user wa…
Dec 21, 2024
93e18aa
The unload utility part will be fixed in issue CBRD-25762.
Dec 21, 2024
c0a67ae
Fixed the memory-monitor-check failure issue. We suspected that a mem…
Dec 23, 2024
c747dfd
edited for readability.
Dec 23, 2024
1f14a07
remove the part that does free_and_init when new_trigger_stmt_str is …
Dec 23, 2024
39e71d3
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Dec 30, 2024
19e79ea
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Dec 30, 2024
3aad5e0
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 2, 2025
3447fc0
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 3, 2025
b24bf76
Reflects the code review content.
Jan 3, 2025
9629c73
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 3, 2025
61101f5
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 6, 2025
a7aaf67
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 6, 2025
815ef05
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 7, 2025
9664301
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 9, 2025
75547f6
Merge remote-tracking branch 'upstream/develop' into CBRD-25719
Jan 9, 2025
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
3 changes: 2 additions & 1 deletion src/object/trigger_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -1668,7 +1668,8 @@ compile_trigger_activity (TR_TRIGGER * trigger, TR_ACTIVITY * activity, int with
class_mop = ((curname == NULL && tempname == NULL) ? NULL : trigger->class_mop);

activity->statement =
pt_compile_trigger_stmt ((PARSER_CONTEXT *) activity->parser, text, class_mop, curname, tempname);
pt_compile_trigger_stmt ((PARSER_CONTEXT *) activity->parser, text, class_mop, curname, tempname,
&activity->source);
if (activity->statement == NULL || pt_has_error ((PARSER_CONTEXT *) activity->parser))
{
error = er_errid ();
Expand Down
93 changes: 91 additions & 2 deletions src/parser/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ static PT_NODE *pt_find_lck_class_from_partition (PARSER_CONTEXT * parser, PT_NO
static int pt_in_lck_array (PT_CLASS_LOCKS * lcks, const char *str, LC_PREFETCH_FLAGS flags);

static void remove_appended_trigger_info (char *msg, int with_evaluate);
static char *change_trigger_action_query (PARSER_CONTEXT * parser, PT_NODE * statement, int with_evaluate);

static PT_NODE *pt_set_trigger_obj_pre (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue_walk);
static PT_NODE *pt_set_trigger_obj_post (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue_walk);
Expand Down Expand Up @@ -1043,6 +1044,74 @@ remove_appended_trigger_info (char *msg, int with_evaluate)
}
}

/*
* change_trigger_action_query () - remove appended trigger info
* parser(in):
* action_stmt(in):
* with_evaluate(in):
*/
static char *
change_trigger_action_query (PARSER_CONTEXT * parser, PT_NODE * statement, int with_evaluate)
ctshim marked this conversation as resolved.
Show resolved Hide resolved
{
int result_len = 0;
char *result = NULL;
char *new_trigger_stmt_str = NULL;
unsigned int save_custom;

assert (parser != NULL || statement != NULL);

save_custom = parser->custom_print;

parser->flag.is_parsing_trigger = 1;

result = parser_print_tree_with_quotes (parser, statement);

parser->flag.is_parsing_trigger = 0;
parser->custom_print = save_custom;

if (result == NULL)
{
return NULL;
}

/* remove appended trigger evaluate info */
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() 함수 내부에서만 사용되도록 수정했습니다.

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);
}
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() 함수를 추가하고, 해당 함수 내부에 반영하여 수정했습니다.


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 () 함수를 추가 했습니다.

}

result_len = strlen (result) + 1;
if (result_len < 0)
{
return NULL;
}

new_trigger_stmt_str = (char *) malloc (result_len);
if (new_trigger_stmt_str == NULL)
{
er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_OUT_OF_VIRTUAL_MEMORY, 1, (size_t) result_len);
return NULL;
}

snprintf (new_trigger_stmt_str, result_len, "%s", result);

return new_trigger_stmt_str;
ctshim marked this conversation as resolved.
Show resolved Hide resolved
}

/*
* pt_compile_trigger_stmt () - Compiles the trigger_stmt so that it can be
* executed by pt_exec_trigger_stmt
Expand All @@ -1057,7 +1126,7 @@ remove_appended_trigger_info (char *msg, int with_evaluate)

PT_NODE *
pt_compile_trigger_stmt (PARSER_CONTEXT * parser, const char *trigger_stmt, DB_OBJECT * class_op, const char *name1,
const char *name2)
const char *name2, char **new_trigger_stmt)
{
char *stmt_str = NULL;
const char *class_name;
Expand All @@ -1066,7 +1135,7 @@ pt_compile_trigger_stmt (PARSER_CONTEXT * parser, const char *trigger_stmt, DB_O
PT_NODE *err_node;
int with_evaluate;

assert (parser != NULL);
assert (parser != NULL || !*new_trigger_stmt);

if (!trigger_stmt)
return NULL;
Expand Down Expand Up @@ -1160,8 +1229,13 @@ pt_compile_trigger_stmt (PARSER_CONTEXT * parser, const char *trigger_stmt, DB_O
upd->info.update.spec = entity;
}

/* prevents forced cast() within the code. (parser->flag.is_parsing_trigger == 1 && p->info.expr.flag != 0) */
parser->flag.is_parsing_trigger = 1;

statement = pt_compile (parser, statement);

parser->flag.is_parsing_trigger = 0;

/* Remove those info we append, which users can't understand them */
if (pt_has_error (parser))
{
Expand All @@ -1177,6 +1251,21 @@ pt_compile_trigger_stmt (PARSER_CONTEXT * parser, const char *trigger_stmt, DB_O
/* We need to do view translation here on the expression to be executed. */
if (statement)
{
char *new_trigger_stmt_str = NULL;

with_evaluate = strstr (trigger_stmt, "EVALUATE ( ") != NULL ? true : false;

new_trigger_stmt_str =
change_trigger_action_query (parser, statement->info.scope.stmt->info.trigger_action.expression, with_evaluate);
if (new_trigger_stmt_str == NULL)
{
return NULL;
}

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.

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


statement->info.scope.stmt->info.trigger_action.expression =
mq_translate (parser, statement->info.scope.stmt->info.trigger_action.expression);
/*
Expand Down
1 change: 1 addition & 0 deletions src/parser/parse_tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -1233,6 +1233,7 @@ parser_create_parser (void)
parser->flag.is_auto_commit = 0;
parser->flag.is_parsing_static_sql = 0;
parser->flag.is_parsing_unload_schema = 0;
parser->flag.is_parsing_trigger = 0;

parser->external_into_label = NULL;
parser->external_into_label_cnt = 0;
Expand Down
1 change: 1 addition & 0 deletions src/parser/parse_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4020,6 +4020,7 @@ struct parser_context
unsigned is_auto_commit:1; /* set to true, if auto commit. */
unsigned is_parsing_static_sql:1; /* For PL/CSQL's static SQL: parameterize PL/CSQL variable symbols (to host variable) */
unsigned is_parsing_unload_schema:1; /* Parsing in unload: used to parse the scode (original query) of PL/CSQL to remove the owner. */
unsigned is_parsing_trigger:1;
} flag;
};

Expand Down
10 changes: 7 additions & 3 deletions src/parser/parse_tree_cl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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

{
q = pt_append_varchar (parser, q, r1);
break;
}

r2 = pt_print_bytes (parser, p->info.expr.cast_type);
q = pt_append_nulstring (parser, q, " cast(");
q = pt_append_varchar (parser, q, r1);
Expand Down Expand Up @@ -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.

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

{
parser->custom_print |= PT_SUPPRESS_RESOLVED;
parser->custom_print & ~PT_PRINT_ALIAS;
Expand Down Expand Up @@ -13576,8 +13582,6 @@ pt_print_name (PARSER_CONTEXT * parser, PT_NODE * p)
PARSER_VARCHAR *q = NULL, *r1;
unsigned int save_custom = parser->custom_print;

char *dot = NULL;

parser->custom_print = parser->custom_print | p->info.name.custom_print;

if (!(parser->custom_print & PT_SUPPRESS_META_ATTR_CLASS) && (p->info.name.meta_class == PT_META_CLASS))
Expand Down
2 changes: 1 addition & 1 deletion src/parser/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ extern "C"
extern PT_NODE *pt_class_pre_fetch (PARSER_CONTEXT * parser, PT_NODE * statement);

extern PT_NODE *pt_compile_trigger_stmt (PARSER_CONTEXT * parser, const char *trigger_stmt, DB_OBJECT * class_op,
const char *name1, const char *name2);
const char *name1, const char *name2, char **new_trigger_stmt);
extern int pt_exec_trigger_stmt (PARSER_CONTEXT * parser, PT_NODE * trigger_stmt, DB_OBJECT * object1,
DB_OBJECT * object2, DB_VALUE * result);

Expand Down
Loading