Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-10142 PL/SQL parser
  3. MDEV-13298

Change sp_head::m_chistics from a pointer to a structure

    Details

    • 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(), ..).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                bar Alexander Barkov
                Reporter:
                bar Alexander Barkov
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: