-
Notifications
You must be signed in to change notification settings - Fork 123
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-25749] Add [NOT DETERMINISTIC | DETERMINISTIC] Keywords to the CREATE FUNCTION Statement #5725
base: develop
Are you sure you want to change the base?
Conversation
src/sp/sp_catalog.hpp
Outdated
@@ -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), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SP_DIRECTIVE_RIGHTS_DETERMINISTIC = (0x02 << 0), | |
SP_DIRECTIVE_DETERMINISTIC = (0x01 << 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.
There was a problem hiding this comment.
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) 에 대한 용어입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인 감사합니다.
다시 수정하여 반영 했습니다.
"NOT DETERMINISTIC" 가 default 라면 unloaddb 할 때 아예 기술하지 않는 것은 어떤가요? |
JIRA 상태를 확인해주세요. |
PL/CSQL의 Lexer 파일 (PlcLexer.g4) 에서 키워드만 추가해서 문법상 통과만 시켜주면 될 것 같습니다. |
… | DETERMINISTIC] keywords to the PL/CSQL Lexer and Parser
@@ -1815,6 +1815,7 @@ namespace cubschema | |||
{"arg_count", "integer"}, | |||
{"lang", "varchar(16)"}, | |||
{"authid", "varchar(16)"}, | |||
{"dtrm_type", "varchar(20)"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"dtrm"을 보고 유추하기 어려울 듯 합니다.
좀 길더라도 "deterministic_" prefix를 사용하는 것이 어떤가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_deterministic varchar(3) 은 어떨까요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
varchar(20) 인 이유는 뭔가요?
여기에 문자열로 담아 두나요?
형규님 의견을 받는다면 Y/N 같은 것으로 충분할 듯요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deterministic 혹은 non_deterministic 으로 저장하기 위한 고려인 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.
src/sp/jsp_cl.cpp
Outdated
@@ -2086,8 +2106,17 @@ jsp_make_pl_signature (PARSER_CONTEXT *parser, PT_NODE *node, PT_NODE *subquery_ | |||
} | |||
|
|||
sig.auth = db_private_strdup (NULL, auth_name); | |||
if ((directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ((directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC)) | |
if (directive & SP_DIRECTIVE_ENUM::SP_DIRECTIVE_RIGHTS_DETERMINISTIC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.
@@ -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 */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deterministic 이 authid 앞에 와도 괜찮도록 문법 구성을 해야 할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀해 주신 부분은 조금 더 코드 분석을 진행한 후 반영하도록 하겠습니다.
@@ -40,7 +40,7 @@ create_routine | |||
|
|||
routine_definition | |||
: (PROCEDURE | FUNCTION) routine_uniq_name ( (LPAREN parameter_list RPAREN)? | LPAREN RPAREN ) (RETURN type_spec)? | |||
(authid_spec)? (IS | AS) (LANGUAGE PLCSQL)? seq_of_declare_specs? body (SEMICOLON)? | |||
(authid_spec)? (deterministic_spec)? (IS | AS) (LANGUAGE PLCSQL)? seq_of_declare_specs? body (SEMICOLON)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
authid_spec 과 deterministic_spec 이 순서가 바뀌어도 에러가 나지 않도록 문법 구성을 해야 할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀해 주신 부분은 조금 더 코드 분석을 진행한 후 반영하도록 하겠습니다.
*/ | ||
if (regu_src->value.sp_ptr->sig->is_deterministic == false) | ||
{ | ||
return ER_FAILED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 assign 문이 실행되지 않도록
27628 이후 추가된 코드는 27627 라인 앞에 위치하는 것이 좋겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
말씀하신 부분을 반영하여 수정했습니다.
*/ | ||
if (regu_src->value.sp_ptr->sig->is_deterministic == false) | ||
{ | ||
return ER_FAILED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[질문] 이 리턴문이 deterministic 하지 않은 function 은 query cache 를 사용하지 못하도록 제약하는 효과가 있는 것인가요.
There was a problem hiding this comment.
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를 사용할 수 없습니다.
src/sp/sp_catalog.hpp
Outdated
@@ -130,6 +130,7 @@ enum sp_directive : int | |||
{ | |||
SP_DIRECTIVE_RIGHTS_OWNER = 0x00, | |||
SP_DIRECTIVE_RIGHTS_CALLER = (0x01 << 0), | |||
SP_DIRECTIVE_RIGHTS_DETERMINISTIC = (0x01 << 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rights 는 owner와 caller 에만 붙는 용어 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
다시 수정하여 반영 했습니다.
src/sp/pl_signature.hpp
Outdated
@@ -92,6 +92,7 @@ namespace cubpl | |||
int type; // PL_TYPE | |||
char *name; | |||
char *auth; | |||
bool is_deterministic; // DETERMINISTIC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pl_signature의 pack, unpack, get_packed_size 에 대해서 is_deterministic 값도 추가 반영해야 XASL과 CALL 문에서 해당 값이 전달됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰 감사합니다.
오프라인으로 주신 피드백을 참고하여 관련된 부분에 아래 조건을 추가했습니다:
#if defined (CS_MODE)
http://jira.cubrid.org/browse/CBRD-25749
Purpose
Stored Procedure Function의 SubQuery Cache를 구분하기 위해 [NOT DETERMINISTIC | DETERMINISTIC] 키워드를 추가합니다.
Implementation
1. CREATE FUNCTION 구문에 [NOT DETERMINISTIC | DETERMINISTIC] 키워드가 추가됩니다.
2. Unload Schema 파일의 CREATE FUNCTION 구문에 [NOT DETERMINISTIC | DETERMINISTIC] 키워드가 추가됩니다.
3. _db_stored_procedure 카탈로그의 directive 컬럼 값에 따른 의미
4. db_stored_procedure 카탈로그 가상 클래스에 dtrm_type 컬럼을 추가합니다.
5. SubQuery에 Stored Procedure Function을 사용하는 경우(correlated), SubQuery Cache가 동작합니다.
Remarks