Re: [Maria-developers] MDEV 18092
Hi Alexander, I was on vacation too, just came back, thank you for teaching me. Indeed it works, here is a patch with quicker solution and the test case *https://github.com/an3l/server/commit/252ca79493e18fa8fd364992f5022af8a57b79... <https://github.com/an3l/server/commit/252ca79493e18fa8fd364992f5022af8a57b79d5>* I added the PR also, so if something needs to be improved please let me know. In addition I will try later also to add `store_schema_params()` function to handler. Thanks, Anel On Mon, Dec 31, 2018 at 7:02 AM Alexander Barkov <bar@mariadb.com> wrote:
Hi Anel,
On 12/28/2018 02:12 PM, Anel Husakovic wrote:
Hi Alexander, my name is Anel, and I'm working for Foundation. I would like to take/ https://jira.mariadb.org/browse/MDEV-18092/ if you allow me, and would appreciate any hint in order to learn more about this codebase :).
Nice to meet you! Thanks for working on this!
I debugged already a bit and have seen that there is parser error, where /*parser_state = m_buf = 0x7fff6403ec90 "CREATE DEFINER=\"\" PACKAGE BODY \"db1\".\"employee_tools\" BEGIN END"/ is raised because the incorrect body of /Sp_handler_package_spec /and/or /Sp_handler_package_body /I suppose ? (/empty_body_lex_cstring/() for *Sp_handler_package_spec *and *Sp_handler_package_body*),
I tried to change the body of the above methods, but either my syntax is not good ("/AS BEGIN NULL; END/") or this is incorrect conclusion :).
It seems Sp_handler::sp_load_for_information_schema() creates a wrong CREATE statement that cannot be parsed.
This should be fixed. sp_load_for_information_schema() is supposed to build a simplified CREATE statement that can be parsed.
But for exactly this problem, it's probably not necessarily.
I think store_schema_params() should just skip PACKAGE and PACKAGE BODY records quickly.
These stored objects do not have any parameters or return values (only procedures and functions have). So no needs to build a CREATE statement and parse it: this won't give us any data useful for INFORMATION_SCHEMA.PARAMETERS anyway.
Ideally, store_schema_params() should be turned into virtual methods in Sp_handler, doing: - empty job for PACKAGE and PACKAGE BODY - adding a return data type record for FUNCTION - adding parameters for FUNCTION and PROCEDURE
A quicker solution would be just to change this block:
if (!sph) DBUG_RETURN(0);
to something like this:
if (!sph || sph->type() == TYPE_ENUM_PACKAGE || sph->type() == TYPE_ENUM_PACKAGE_BODY) DBUG_RETURN(0);
<skip>
Thanks in advance and happy holidays,
Thanks! Same to you!
I'll be on holidays until the 10th of January. Sorry for possible slow replies during these days.
Anel
Hi Anel, On 01/03/2019 11:43 PM, Anel Husakovic wrote:
Hi Alexander, I was on vacation too, just came back, thank you for teaching me. Indeed it works, here is a patch with quicker solution and the test case _https://github.com/an3l/server/commit/252ca79493e18fa8fd364992f5022af8a57b79...
Please see my last comment here: https://jira.mariadb.org/browse/MDEV-18092 Thanks!
I added the PR also, so if something needs to be improved please let me know. In addition I will try later also to add `store_schema_params()` function to handler.
Thanks, Anel
On Mon, Dec 31, 2018 at 7:02 AM Alexander Barkov <bar@mariadb.com <mailto:bar@mariadb.com>> wrote:
Hi Anel,
On 12/28/2018 02:12 PM, Anel Husakovic wrote: > Hi Alexander, > my name is Anel, and I'm working for Foundation. > I would like to take/ https://jira.mariadb.org/browse/MDEV-18092/ if > you allow me, and would appreciate any hint in order to learn more about > this codebase :).
Nice to meet you! Thanks for working on this!
> I debugged already a bit and have seen that there is parser error, > where /*parser_state = m_buf = 0x7fff6403ec90 "CREATE DEFINER=\"\" > PACKAGE BODY \"db1\".\"employee_tools\" BEGIN END"/ > is raised because the incorrect body of /Sp_handler_package_spec > /and/or /Sp_handler_package_body /I suppose ? > (/empty_body_lex_cstring/() for *Sp_handler_package_spec *and > *Sp_handler_package_body*), > > I tried to change the body of the above methods, but either my syntax is > not good ("/AS BEGIN NULL; END/") or this is incorrect conclusion :).
It seems Sp_handler::sp_load_for_information_schema() creates a wrong CREATE statement that cannot be parsed.
This should be fixed. sp_load_for_information_schema() is supposed to build a simplified CREATE statement that can be parsed.
But for exactly this problem, it's probably not necessarily.
I think store_schema_params() should just skip PACKAGE and PACKAGE BODY records quickly.
These stored objects do not have any parameters or return values (only procedures and functions have). So no needs to build a CREATE statement and parse it: this won't give us any data useful for INFORMATION_SCHEMA.PARAMETERS anyway.
Ideally, store_schema_params() should be turned into virtual methods in Sp_handler, doing: - empty job for PACKAGE and PACKAGE BODY - adding a return data type record for FUNCTION - adding parameters for FUNCTION and PROCEDURE
A quicker solution would be just to change this block:
if (!sph) DBUG_RETURN(0);
to something like this:
if (!sph || sph->type() == TYPE_ENUM_PACKAGE || sph->type() == TYPE_ENUM_PACKAGE_BODY) DBUG_RETURN(0);
<skip>
> Thanks in advance and happy holidays,
Thanks! Same to you!
I'll be on holidays until the 10th of January. Sorry for possible slow replies during these days.
> > Anel
participants (2)
-
Alexander Barkov -
Anel Husakovic