Details
-
Technical task
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
-
10.3(EOL)
-
10.2.2-3, 10.2.2-1, 10.2.2-2, 10.2.2-4, 10.1.18
Description
In order to reuse sp_head for new classes behind Oracle-style packages (MDEV-10591), we'll fix some st_sp_chistics related design problems in sp_head.
1. sp_head has a member st_sp_chistics *m_chistics, which can point in different moments of time to memory of different nature:
- m_chistics can be NULL
- m_chistics can point to Lex->sp_chistics
- m_chistics can point to a memory allocated on sp_head::mem_root
- m_chistics.comment.str can be NULL
- m_chistics.comment.str can point to the currently parsed query fragment (the query itself is alloced on thd->mem_root).
- m_chistics.comment.str can point to a memory allocated on sp_head::mem_root
This is very hard to follow and is bug prone.
This is the reason of some redundant code (a developer can never be sure which memory type m_chistics or its component point to, and has to do extra safety initialization/copying).
2. Having st_sp_chistics *m_chistics (as a pointer) rather than st_sp_chistics m_chistics (as a structure) is simply pointless, because:
- Any sp_head instance sooneer or later ends up in allocating an instance of st_sp_chistics on its own mem_root and assigning a pointer to it to m_chistics.
- Life cycle of m_chistics[0] ends up in exactly the same point of time when the owner sp_head itself is freed.
- Non of sp_head instances share the same sp_sp_chistics.
We'll do the following:
1. Change the data type of sp_head::m_chistics from a pointer to a structure
2. Move m_chistics to the private section, to disallow initialization of its components to arbitrary memory types. sp_head::set_info() will make sure that m_chistics.comment.str is set either to NULL, or to a memory allocated on sp_head::mem_root.
3. Introduce a few methods to access m_chistics components for read and write. Btw, this will also make the calling code shorter and easier to read.
4. Fix the parser to set sp_head::m_chistics immediately after scanning the sp_c_chistics rule. This is much easier to read (comparing to a postponed initialization, from Lex->sp_chistics).
5. Add initialization of sp_head::m_created and sp_head::m_modified at contructor time. This is much reasier to follow (compating to a postponed initialization, e.g. by set_info())).
6. Remove redundant code.
- Event_job_data::execute() does not have to do full set_info(). It's enough to set m_sql_mode only. Other m_chistics components were re-assigned to the default values again.
- In the new design, LEX::make_sp_head() does not have to do the hack with assigning sp->m_chistics to &Lex->sp_chistics.
- The ev_sql_stmt rule does not have to assing lex->sp_chistics.suid to SP_IS_SUID. This value is never used later.
7. Introduce a new class Sp_chistics, derived from st_sp_chistics, with automatic initialization. Reuse this class instead of st_sp_chistics when applicable, and remove tones of duplicate bzero's.
8. Fix show_create_sp() to accept st_sp_chistics as a reference, rather than as a pointer:
- to reduce changes in the code passing sp_head::m_chistics
- for consistency with db_load_routine()
- to be able to pass temporary on-stack Sp_chistics instances, e.g. show_create_sp(.., Sp_chistics(), ..).
Attachments
Issue Links
- blocks
-
MDEV-10591 Oracle-style packages
- Closed