Hello Sergei,
Thanks for the review.
The new patch is here:
https://github.com/MariaDB/server/commit/a5344196e01239769bbf214ced0205178001c49f
It removes the two redundant calls:
pkg->set_c_chistics(Lex->sp_chistics)
Please find comments below.
On 11/7/23 10:39 PM, Sergei Golubchik wrote:
Hi, Alexander,
Looks great! Just a couple of questions below
On Nov 07, Alexander Barkov wrote:
revision-id: 28deba0a611 (mariadb-11.0.1-239-g28deba0a611)
parent(s): e9573c05965
author: Alexander Barkov
committer: Alexander Barkov
timestamp: 2023-09-22 14:38:52 +0400
message:
MDEV-32101 CREATE PACKAGE [BODY] for sql_mode=DEFAULT
This patch adds PACKAGE support with SQL/PSM dialect for sql_mode=DEFAULT:
- CREATE PACKAGE
- DROP PACKAGE
- CREATE PACKAGE BODY
- DROP PACKAGE BODY
- Package function and procedure invocation from outside of the package:
-- using two step identifiers
SELECT pkg.f1();
CALL pkg.p1()
-- using three step identifiers
SELECT db.pkg.f1();
CALL db.pkg.p1();
This is a non-standard MariaDB extension.
However, later this code can be used to implement
the SQL Standard and DB2 dialects of CREATE MODULE.
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index af2ec678daa..fca39b63312 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -1285,7 +1285,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t
*yystacksize);
%left TRANSACTION_SYM TIMESTAMP PERIOD_SYM SYSTEM USER COMMENT_SYM
%left PREC_BELOW_SP_OBJECT_TYPE
-%left FUNCTION_SYM
+%left PACKAGE_MARIADB_SYM FUNCTION_SYM
Why PACKAGE_ORACLE_SYM and PACKAGE_ALLMODES_SYM don't need it?
(same for BODY_ORACLE_SYM and BODY_ALLMODES_SYM)
Keywords PACKAGE and BODY have different "reserved-ness"
for sql_mode=DEFAULT and sql_mode=ORACLE.
For sql_mode=ORACLE, they are more reserved and can
act as identifiers only in the rule
"reserved_keyword_udt_not_param_type".
The grammar is non-conflicting, so
"PACKAGE" does not need any tuning with precedence.
For sql_mode=DEFAULT, for backward compatibility,
they can act as identifiers in the rule
"keyword_sp_var_and_label". So here we need to tell
the parser that "PACKAGE" is an identifier, but only
when it is not followed by "BODY".
BODY itself does not need precedence tuning.
/*
@@ -3049,6 +3059,8 @@ sp_handler:
| PROCEDURE_SYM { $$= &sp_handler_procedure; }
| PACKAGE_ORACLE_SYM { $$= &sp_handler_package_spec; }
| PACKAGE_ORACLE_SYM BODY_ORACLE_SYM { $$= &sp_handler_package_body; }
+ | PACKAGE_MARIADB_SYM { $$= &sp_handler_package_spec; }
+ | PACKAGE_MARIADB_SYM BODY_MARIADB_SYM { $$= &sp_handler_package_body;
}
Why not to use here PACKAGE_ALLMODES_SYM and BODY_ALLMODES_SYM?
(also, I'd personally call them simply PACKAGE_SYM and BODY_SYM)
I checked - this will cause one more shift/reduce :(
;
@@ -19428,35 +19489,33 @@ create_routine:
(Item_result) $8, $10))
MYSQL_YYABORT;
}
- | create_or_replace definer_opt PACKAGE_ORACLE_SYM
+ | create_or_replace definer_opt PACKAGE_ALLMODES_SYM
opt_if_not_exists sp_name opt_create_package_chistics_init
{
sp_package *pkg;
if (unlikely(!(pkg= Lex->
- create_package_start(thd,
- SQLCOM_CREATE_PACKAGE,
- &sp_handler_package_spec,
- $5, $1 | $4))))
+ create_package_start(thd, &sp_handler_package_spec,
+ $5, $1 | $4,
+ Lex->sp_chistics))))
MYSQL_YYABORT;
pkg->set_c_chistics(Lex->sp_chistics);
Looks redundant
Thanks. Good catch. Fixed.
Lex->sphead->set_body_start(thd, YYLIP->get_cpp_tok_start());
}
sp_tail_is
opt_package_specification_element_list END
- remember_end_opt opt_sp_name
+ remember_end_opt opt_trailing_sp_name
{
if (unlikely(Lex->create_package_finalize(thd, $5, $12, $11)))
MYSQL_YYABORT;
}
- | create_or_replace definer_opt PACKAGE_ORACLE_SYM BODY_ORACLE_SYM
+ | create_or_replace definer_opt PACKAGE_ALLMODES_SYM BODY_ALLMODES_SYM
opt_if_not_exists sp_name opt_create_package_chistics_init
{
sp_package *pkg;
if (unlikely(!(pkg= Lex->
- create_package_start(thd,
- SQLCOM_CREATE_PACKAGE_BODY,
- &sp_handler_package_body,
- $6, $1 | $5))))
+ create_package_start(thd, &sp_handler_package_body,
+ $6, $1 | $5,
+ Lex->sp_chistics))))
MYSQL_YYABORT;
pkg->set_c_chistics(Lex->sp_chistics);
same
Fixed.
Lex->sphead->set_body_start(thd, YYLIP->get_cpp_tok_start());
Regards,
Sergei
Chief Architect, MariaDB Server
and secur...@mariadb.org
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org