PL/SQL parser (MDEV-10142)

[MDEV-13298] Change sp_head::m_chistics from a pointer to a structure Created: 2017-07-12  Updated: 2018-08-31  Resolved: 2017-07-12

Status: Closed
Project: MariaDB Server
Component/s: Stored routines
Affects Version/s: 10.3
Fix Version/s: 10.3.1

Type: Technical task Priority: Major
Reporter: Alexander Barkov Assignee: Alexander Barkov
Resolution: Fixed Votes: 0
Labels: refactoring

Issue Links:
Blocks
blocks MDEV-10591 Oracle-style packages Closed
Sprint: 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(), ..).


 Comments   
Comment by Alexander Barkov [ 2017-07-12 ]

Pushed to bb-10.2-ext

Generated at Thu Feb 08 08:04:29 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.