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.

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

[CBRD-25749] Add [NOT DETERMINISTIC | DETERMINISTIC] Keywords to the CREATE FUNCTION Statement #5725

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

[CBRD-25749] Add [NOT DETERMINISTIC | DETERMINISTIC] Keywords to the CREATE FUNCTION Statement #5725

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2260c0b
Add [NOT DETERMINISTIC | DETERMINISTIC] Keywords to the CREATE FUNCTI…
Dec 17, 2024
0542c3f
Merge remote-tracking branch 'upstream/develop' into CBRD-25749
Dec 17, 2024
cd4e6c3
1. Modify based on code review feedback, 2.Add the [NOT DETERMINISTIC…
Dec 17, 2024
71ee4cc
Add the dtrm_type column to the db_stored_procedure catalog view table.
Dec 18, 2024
0fcb37d
Merge remote-tracking branch 'upstream/develop' into CBRD-25749
Dec 20, 2024
ca10800
Reflects some of the code review content.
Dec 21, 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
13 changes: 13 additions & 0 deletions src/executables/unload_schema.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4531,6 +4531,19 @@ emit_stored_procedure_pre (extract_context & ctxt, print_output & output_ctx)
output_ctx ("AUTHID OWNER ");
}

// dtrm_type
if (sp_type == SP_TYPE_FUNCTION)
{
if (directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC)
{
output_ctx ("DETERMINISTIC ");
}
else
{
output_ctx ("NOT DETERMINISTIC ");
}
}

int sp_lang = db_get_int (&lang_val);
if (sp_lang == SP_LANG_PLCSQL)
{
Expand Down
64 changes: 49 additions & 15 deletions src/parser/csql_grammar.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ static int g_plcsql_text_pos;
%type <number> deduplicate_key_mod_level
%type <number> opt_index_with_clause_no_online
%type <number> opt_authid
%type <number> opt_deterministic
/*}}}*/

/* define rule type (node) */
Expand Down Expand Up @@ -1553,6 +1554,7 @@ static int g_plcsql_text_pos;
%token <cptr> DECREMENT
%token <cptr> DEFINER
%token <cptr> DENSE_RANK
%token <cptr> DETERMINISTIC
%token <cptr> DONT_REUSE_OID
%token <cptr> ELT
%token <cptr> EMPTY
Expand Down Expand Up @@ -3129,14 +3131,15 @@ create_stmt
opt_sp_param_list /* 6 */
RETURN sp_return_type /* 7, 8 */
opt_authid /* 9 */
is_or_as pl_language_spec /* 10, 11 */
opt_comment_spec /* 12 */
opt_deterministic /* 10 */
Copy link
Contributor

Choose a reason for hiding this comment

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

deterministic 이 authid 앞에 와도 괜찮도록 문법 구성을 해야 할 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

코드 리뷰 감사합니다.

말씀해 주신 부분은 조금 더 코드 분석을 진행한 후 반영하도록 하겠습니다.

is_or_as pl_language_spec /* 11, 12 */
opt_comment_spec /* 13 */
{ pop_msg(); }
{{ DBG_TRACE_GRAMMAR(create_stmt, | CREATE opt_or_replace FUNCTION~);
PT_NODE *node = parser_pop_hint_node ();
if (node)
{
PT_NODE* body = $11;
PT_NODE* body = $12;
if (body->info.sp_body.lang == SP_LANG_PLCSQL && body->info.sp_body.impl == NULL)
{
// In particular, this happens for two cases:
Expand All @@ -3148,8 +3151,8 @@ create_stmt
assert(this_parser->file);

int start = @1.buffer_pos - 6; // 6 : length of "create"
int spec_start = @10.buffer_pos; // right after is_or_as
int spec_end = @11.buffer_pos;
int spec_start = @11.buffer_pos; // right after is_or_as
int spec_end = @12.buffer_pos;
int end = @$.buffer_pos;
if (pt_set_plcsql_body_impl(node, body, start, spec_start, spec_end, end) < 0) {
PT_ERROR (this_parser, node, "failed to get the user SQL from the input file");
Expand All @@ -3160,11 +3163,12 @@ create_stmt
node->info.sp.name = $5;
node->info.sp.type = PT_SP_FUNCTION;
node->info.sp.auth_id = $9;
node->info.sp.dtrm_type = $10;
node->info.sp.param_list = $6;
node->info.sp.ret_type = (int) TO_NUMBER(CONTAINER_AT_0($8));
node->info.sp.ret_data_type = CONTAINER_AT_1($8);
node->info.sp.body = $11;
node->info.sp.comment = $12;
node->info.sp.body = $12;
node->info.sp.comment = $13;
}

$$ = node;
Expand Down Expand Up @@ -12833,25 +12837,54 @@ sp_return_type

opt_authid
: /* empty */
{{ $$ = PT_AUTHID_OWNER; }}
{{ DBG_TRACE_GRAMMAR(opt_authid, : );
$$ = PT_AUTHID_OWNER;
DBG_PRINT}}
| AUTHID DEFINER
{{ $$ = PT_AUTHID_OWNER; }}
{{ DBG_TRACE_GRAMMAR(opt_authid, : AUTHID DEFINER);
$$ = PT_AUTHID_OWNER;
DBG_PRINT}}
| AUTHID OWNER
{{ $$ = PT_AUTHID_OWNER; }}
{{ DBG_TRACE_GRAMMAR(opt_authid, : AUTHID OWNER);
$$ = PT_AUTHID_OWNER;
DBG_PRINT}}
| AUTHID CALLER
{{ $$ = PT_AUTHID_CALLER; }}
{{ DBG_TRACE_GRAMMAR(opt_authid, : AUTHID CALLER);
$$ = PT_AUTHID_CALLER;
DBG_PRINT}}
| AUTHID CURRENT_USER
{{ $$ = PT_AUTHID_CALLER; }}
{{ DBG_TRACE_GRAMMAR(opt_authid, : AUTHID CURRENT_USER);
$$ = PT_AUTHID_CALLER;
DBG_PRINT}}
;

opt_deterministic
: /* empty */
{{ DBG_TRACE_GRAMMAR(opt_deterministic, : );
$$ = PT_NOT_DETERMINISTIC;
DBG_PRINT}}
| NOT DETERMINISTIC
{{ DBG_TRACE_GRAMMAR(opt_deterministic, : NOT DETERMINISTIC);
$$ = PT_NOT_DETERMINISTIC;
DBG_PRINT}}
| DETERMINISTIC
{{ DBG_TRACE_GRAMMAR(opt_deterministic, : DETERMINISTIC);
$$ = PT_DETERMINISTIC;
DBG_PRINT}}
;

is_or_as
: IS
| AS
: IS
{ DBG_TRACE_GRAMMAR(is_or_as, : IS); DBG_PRINT}
| AS
{ DBG_TRACE_GRAMMAR(is_or_as, : AS); DBG_PRINT}
;

opt_lang_plcsql
: /* empty */
: /* empty */
{ DBG_TRACE_GRAMMAR(opt_lang_plcsql, : ); DBG_PRINT}
| LANGUAGE PLCSQL
{ DBG_TRACE_GRAMMAR(opt_lang_plcsql, : LANGUAGE PLCSQL); DBG_PRINT}
;

pl_language_spec
Expand Down Expand Up @@ -23217,6 +23250,7 @@ identifier
| DEFINER {{ DBG_TRACE_GRAMMAR(identifier, | DEFINER ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| DEDUPLICATE_ {{ DBG_TRACE_GRAMMAR(identifier, | DEDUPLICATE_ ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| DENSE_RANK {{ DBG_TRACE_GRAMMAR(identifier, | DENSE_RANK ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| DETERMINISTIC {{ DBG_TRACE_GRAMMAR(identifier, | DETERMINISTIC ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| DISK_SIZE {{ DBG_TRACE_GRAMMAR(identifier, | DISK_SIZE ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| DONT_REUSE_OID {{ DBG_TRACE_GRAMMAR(identifier, | DONT_REUSE_OID ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
| ELT {{ DBG_TRACE_GRAMMAR(identifier, | ELT ); SET_CPTR_2_PTNAME($$, $1, @$.buffer_pos); }}
Expand Down
7 changes: 5 additions & 2 deletions src/parser/csql_lexer.l
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ IDL [a-zA-Z0-9_]
[dD][eE][sS][cC] { begin_token(yytext); return DESC; }
[dD][eE][sS][cC][rR][iI][bB][eE] { begin_token(yytext); return DESCRIBE; }
[dD][eE][sS][cC][rR][iI][pP][tT][oO][rR] { begin_token(yytext); return DESCRIPTOR; }
[dD][eE][tT][eE][rR][mM][iI][nN][iI][sS][tT][iI][cC] { begin_token(yytext);
csql_yylval.cptr = pt_makename(yytext);
return DETERMINISTIC; }
[dD][iI][aA][gG][nN][oO][sS][tT][iI][cC][sS] { begin_token(yytext); return DIAGNOSTICS; }
[dD][iI][fF][fF][eE][rR][eE][nN][cC][eE] { begin_token(yytext); return DIFFERENCE_; }
[dD][iI][sS][cC][oO][nN][nN][eE][cC][tT] { begin_token(yytext); return DISCONNECT; }
Expand Down Expand Up @@ -722,8 +725,8 @@ IDL [a-zA-Z0-9_]
[oO][vV][eE][rR] { begin_token(yytext); return OVER; }
[oO][vV][eE][rR][lL][aA][pP][sS] { begin_token(yytext); return OVERLAPS; }
[oO][wW][nN][eE][rR] { begin_token(yytext);
csql_yylval.cptr = pt_makename(yytext);
return OWNER; }
csql_yylval.cptr = pt_makename(yytext);
return OWNER; }
[pP][aA][gG][eE] { begin_token(yytext);
csql_yylval.cptr = pt_makename(yytext);
return PAGE; }
Expand Down
1 change: 1 addition & 0 deletions src/parser/keyword.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ static KEYWORD_RECORD keywords[] = {
{DESC, "DESC", 0},
{DESCRIBE, "DESCRIBE", 0},
{DESCRIPTOR, "DESCRIPTOR", 0},
{DETERMINISTIC, "DETERMINISTIC", 1},
{DIAGNOSTICS, "DIAGNOSTICS", 0},
{DIFFERENCE_, "DIFFERENCE", 0},
{DISCONNECT, "DISCONNECT", 0},
Expand Down
5 changes: 4 additions & 1 deletion src/parser/parse_tree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,9 @@ typedef enum
PT_SYNONYM,

PT_AUTHID_OWNER,
PT_AUTHID_CALLER
PT_AUTHID_CALLER,
PT_NOT_DETERMINISTIC,
PT_DETERMINISTIC
// todo: separate into relevant enumerations
} PT_MISC_TYPE;

Expand Down Expand Up @@ -3402,6 +3404,7 @@ struct pt_stored_proc_info
PT_NODE *comment;
PT_NODE *owner; /* for ALTER PROCEDURE/FUNCTION name OWNER TO new_owner */
PT_MISC_TYPE auth_id; /* PT_AUTHID_OWNER, PT_AUTHID_CALLER */
PT_MISC_TYPE dtrm_type; /* PT_NOT_DETERMINISTIC, PT_DETERMINISTIC */
PT_MISC_TYPE type;
unsigned or_replace:1; /* OR REPLACE clause */
PT_TYPE_ENUM ret_type;
Expand Down
12 changes: 12 additions & 0 deletions src/parser/parse_tree_cl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7655,6 +7655,18 @@ pt_print_create_stored_procedure (PARSER_CONTEXT * parser, PT_NODE * p)
{
q = pt_append_nulstring (parser, q, " AUTHID CALLER");
}

if (p->info.sp.type == PT_SP_FUNCTION)
{
if (p->info.sp.dtrm_type == PT_NOT_DETERMINISTIC)
{
q = pt_append_nulstring (parser, q, " NOT DETERMINISTIC");
}
else
{
q = pt_append_nulstring (parser, q, " DETERMINISTIC");
}
}
}

r3 = pt_print_bytes (parser, p->info.sp.body);
Expand Down
13 changes: 13 additions & 0 deletions src/parser/xasl_generation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
#include "jsp_cl.h"
#include "subquery_cache.h"
#include "pl_signature.hpp"
#include "sp_catalog.hpp"

#if defined(WINDOWS)
#include "wintcp.h"
Expand Down Expand Up @@ -27624,6 +27625,18 @@ pt_make_sq_cache_key_struct (QPROC_DB_VALUE_LIST key_struct, void *p, int type)
break;
case TYPE_SP:
regu_var_list_p = regu_src->value.sp_ptr->args;

/* The value of regu_src->value.sp_ptr.sig.dtrm is interpreted as follows
* 0: PT_AUTHID_OWNER + PT_NOT_DETERMINISTIC
* 1: PT_AUTHID_CALLER + PT_NOT_DETERMINISTIC
* 2: PT_AUTHID_OWNER + PT_DETERMINISTIC
* 3: PT_AUTHID_CALLER + PT_DETERMINISTIC
*/
if (!(regu_src->value.sp_ptr->sig->dtrm & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC))
{
return ER_FAILED;
Copy link
Contributor

@hyunikn hyunikn Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

불필요한 assign 문이 실행되지 않도록
27628 이후 추가된 코드는 27627 라인 앞에 위치하는 것이 좋겠습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

[질문] 이 리턴문이 deterministic 하지 않은 function 은 query cache 를 사용하지 못하도록 제약하는 효과가 있는 것인가요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네, 맞습니다.

deterministic 키워드를 사용한 function만 아래 로직을 수행하여 query cache 사용할 수 있습니다.
반대로, deterministic 키워드가 없는 함수는 아래 로직을 수행하지 않으므로 query cache를 사용할 수 없습니다.

}

while (regu_var_list_p)
{
regu_src = &regu_var_list_p->value;
Expand Down
25 changes: 23 additions & 2 deletions src/sp/jsp_cl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@
#define PT_NODE_SP_AUTHID(node) \
((node)->info.sp.auth_id)

#define PT_NODE_SP_DETERMINISTIC_TYPE(node) \
((node)->info.sp.dtrm_type)

#define PT_NODE_SP_COMMENT(node) \
(((node)->info.sp.comment == NULL) ? "" : \
(char *) (node)->info.sp.comment->info.value.data_value.str->bytes)
Expand All @@ -124,6 +127,7 @@ static SP_TYPE_ENUM jsp_map_pt_misc_to_sp_type (PT_MISC_TYPE pt_enum);
static SP_MODE_ENUM jsp_map_pt_misc_to_sp_mode (PT_MISC_TYPE pt_enum);
static PT_MISC_TYPE jsp_map_sp_type_to_pt_misc (SP_TYPE_ENUM sp_type);
static SP_DIRECTIVE_ENUM jsp_map_pt_to_sp_authid (PT_MISC_TYPE pt_authid);
static SP_DIRECTIVE_ENUM jsp_map_pt_to_sp_dtrm_type (PT_MISC_TYPE pt_dtrm_type, SP_DIRECTIVE_ENUM directive);

static char *jsp_check_stored_procedure_name (const char *str);
static int jsp_check_overflow_args (PARSER_CONTEXT *parser, PT_NODE *node, int num_params, int num_args);
Expand Down Expand Up @@ -1012,6 +1016,8 @@ jsp_create_stored_procedure (PARSER_CONTEXT *parser, PT_NODE *statement)
if (sp_info.sp_type == SP_TYPE_FUNCTION)
{
sp_info.return_type = pt_type_enum_to_db (statement->info.sp.ret_type);
// check the deterministic_type of function
sp_info.directive = jsp_map_pt_to_sp_dtrm_type (PT_NODE_SP_DETERMINISTIC_TYPE (statement), sp_info.directive);
}
else
{
Expand Down Expand Up @@ -1477,6 +1483,20 @@ jsp_map_pt_to_sp_authid (PT_MISC_TYPE pt_authid)
SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_CALLER);
}

static SP_DIRECTIVE_ENUM
jsp_map_pt_to_sp_dtrm_type (PT_MISC_TYPE pt_dtrm_type, SP_DIRECTIVE_ENUM directive)
{
assert (pt_dtrm_type == PT_NOT_DETERMINISTIC || pt_dtrm_type == PT_DETERMINISTIC);

if (pt_dtrm_type == PT_DETERMINISTIC)
{
directive = static_cast<SP_DIRECTIVE_ENUM> (static_cast<int> (directive) | static_cast<int>
(SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC));
}

return directive;
}

/*
* jsp_check_stored_procedure_name -
* return: java stored procedure name
Expand Down Expand Up @@ -2066,7 +2086,7 @@ jsp_make_pl_signature (PARSER_CONTEXT *parser, PT_NODE *node, PT_NODE *subquery_

/* semantic check */
int directive = db_get_int (&entry.vals[SP_ATTR_INDEX_DIRECTIVE]);
const char *auth_name = (directive == SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_OWNER ? jsp_get_owner_name (
const char *auth_name = (! (directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_CALLER) ? jsp_get_owner_name (
name, user_name_buffer, DB_MAX_USER_LENGTH) : au_get_current_user_name ());

int result_type = db_get_int (&entry.vals[SP_ATTR_INDEX_RETURN_TYPE]);
Expand All @@ -2086,8 +2106,9 @@ jsp_make_pl_signature (PARSER_CONTEXT *parser, PT_NODE *node, PT_NODE *subquery_
}

sig.auth = db_private_strdup (NULL, auth_name);
sig.dtrm = directive;
sig.result_type = result_type;
if (directive == SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_OWNER)
if (! (directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_CALLER))
{
jsp_get_owner_name (name, user_name_buffer, DB_MAX_USER_LENGTH);
sig.auth = db_private_strndup (NULL, user_name_buffer, DB_MAX_USER_LENGTH);
Expand Down
1 change: 1 addition & 0 deletions src/sp/pl_signature.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ namespace cubpl
int type; // PL_TYPE
char *name;
char *auth;
int dtrm; // DETERMINISTIC
ctshim marked this conversation as resolved.
Show resolved Hide resolved
int result_type; // DB_TYPE

pl_arg arg;
Expand Down
1 change: 1 addition & 0 deletions src/sp/sp_catalog.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ enum sp_directive : int
{
SP_DIRECTIVE_RIGHTS_OWNER = 0x00,
SP_DIRECTIVE_RIGHTS_CALLER = (0x01 << 0),
SP_DIRECTIVE_RIGHTS_DETERMINISTIC = (0x02 << 0),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SP_DIRECTIVE_RIGHTS_DETERMINISTIC = (0x02 << 0),
SP_DIRECTIVE_DETERMINISTIC = (0x01 << 1),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

RIGHTS가 제거되지 않았습니다. RIGHTS는 Execution Rights 관련 문법 (DEFINER, CURRENT_USER) 에 대한 용어입니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

확인 감사합니다.

다시 수정하여 반영 했습니다.

};
typedef sp_directive SP_DIRECTIVE_ENUM;

Expand Down
Loading