Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-5443

Consistent use of the HA_EXTRA_KEYREAD mode (covering index scan)

    Details

    • Type: Task
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Fix Version/s: 10.1
    • Component/s: None
    • Labels:

      Description

      This task is inspired by MDEV-5399. There, TokuDB got confused because the query engine first said that it will do an index-covering scan, but then it did a table scan, so TokuDB didn't produce any data.

      It makes sense to make the query engine be more strict with respect to what it asks from the storage engine (SE). Currently the query engine assumes that the SE would ignore the keyread mode when a table scan is opened. Currently the query engine uses two separate booleans to specify what kind of access to do with respect to index scans:

      • bool TABLE::key_read
      • bool TABLE::no_keyread

      A proposal from Serg is to:

      • Replace the two flags above with an enum that specifies if a keyread is possible, and if it should be done on the next index access:
        • enum covering_scan {NEVER, POSSIBLE, CHOSEN, ENABLED}
        • NEVER - do not use a covering index scan either because it is not possible, or because we don't want it. We do not want to use an index-only scan for instance for delete/update statements.
        • POSSIBLE - index-only scan is possible to use.
        • CHOSEN - the optimizer decided to use an index-only scan when a particular index is scanned.
        • ENABLED - the execution engine should perform the scan as index-only.
      • Remove file->extra(HA_EXTRA_KEYREAD) from everywhere in the source code, and add:
        • one file->extra(HA_EXTRA_KEYREAD) into handler::ha_index_init, and
        • one file->extra(HA_NO_EXTRA_KEYREAD) into handler::ha_rnd_init
          (both look at this TABLE enum variable and change it accordingly)
      • probably, remove all file->extra(HA_NO_EXTRA_KEYREAD) and add one to ha_external_lock(F_UNLCK)

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timour Timour Katchaounov added a comment -

            Two questions re the above idea:

            • If we use NEVER to disable index-only scans, when they are actually possible, we loose the information that they were possible, so if needed, we cannot enable back an index-only scan. Most likely this will never be needed, but this becomes implicit knowledge that follows from the execution logic.
            • It is not clear how does the index-only instruction work in the case of index merge. There we have one TABLE object, with one state of the index scan, however we may merge two different range scans, where one is index-only and the other isn't. Is this possible at all? How does the range optimizer handle this situation?
            Show
            timour Timour Katchaounov added a comment - Two questions re the above idea: If we use NEVER to disable index-only scans, when they are actually possible, we loose the information that they were possible, so if needed, we cannot enable back an index-only scan. Most likely this will never be needed, but this becomes implicit knowledge that follows from the execution logic. It is not clear how does the index-only instruction work in the case of index merge. There we have one TABLE object, with one state of the index scan, however we may merge two different range scans, where one is index-only and the other isn't. Is this possible at all? How does the range optimizer handle this situation?
            Hide
            timour Timour Katchaounov added a comment -

            An alternative solution to the one with enums above is to keep the two booleans.

            <serg> in which case there's no need to do an enum at all, and the fix could be as simple as moving
            <serg> if (table->covering_keys.is_set(tab->ref.key) && !table->no_keyread)
            <serg> table->enable_keyread();
            <serg> inside handler::ha_index_init()
            <serg> well, then it'll be
            <serg> if (table->covering_keys.is_set(idx) && !table->no_keyread && !table->keyread)
            <serg> table->enable_keyread();
            <serg>
            <serg> you cannot access both at the same time via the same handler

            Show
            timour Timour Katchaounov added a comment - An alternative solution to the one with enums above is to keep the two booleans. <serg> in which case there's no need to do an enum at all, and the fix could be as simple as moving <serg> if (table->covering_keys.is_set(tab->ref.key) && !table->no_keyread) <serg> table->enable_keyread(); <serg> inside handler::ha_index_init() <serg> well, then it'll be <serg> if (table->covering_keys.is_set(idx) && !table->no_keyread && !table->keyread) <serg> table->enable_keyread(); <serg> <serg> you cannot access both at the same time via the same handler

              People

              • Assignee:
                Unassigned
                Reporter:
                timour Timour Katchaounov
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: