Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outline digest match fails with EXPLAIN ANALYZE #46

Open
kaiwangchen opened this issue Jan 17, 2025 · 1 comment
Open

Outline digest match fails with EXPLAIN ANALYZE #46

kaiwangchen opened this issue Jan 17, 2025 · 1 comment

Comments

@kaiwangchen
Copy link

-- ind_1 was chosen without outline
CALL dbms_outln.add_index_outline('outline_db', '', 1, 'USE INDEX', 'ind_2', '',
"select * from t1 where t1.col1 =1 and t1.col2 ='xpchild'");
EXPLAIN FORMAT = tree SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
EXPLAIN
-> Filter: (t1.col1 = 2)  (cost=0.35 rows=1)
    -> Index lookup on t1 using ind_2 (col2='xpchild')  (cost=0.35 rows=1)

EXPLAIN ANALYZE SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
EXPLAIN
-> Filter: (t1.col2 = 'xpchild')  (cost=0.35 rows=1) (actual time=0.016..0.017 rows=1 loops=1)
    -> Index lookup on t1 using ind_1 (col1=2)  (cost=0.35 rows=1) (actual time=0.014..0.016 rows=1 loops=1)
@kaiwangchen
Copy link
Author

diff --git a/mysql-test/suite/galaxy_sql/r/feature_outline.result b/mysql-test/suite/galaxy_sql/r/feature_outline.result
index 75dbd183931..0ecde35091d 100644
--- a/mysql-test/suite/galaxy_sql/r/feature_outline.result
+++ b/mysql-test/suite/galaxy_sql/r/feature_outline.result
@@ -541,6 +541,26 @@ id select_type     table   partitions      type    possible_keys   key     key_len ref     rows    filtered
 1      SIMPLE  t1      NULL    ref     ind_1   ind_1   5       const   1       100.00  Using where
 Warnings:
 Note   1003    /* select#1 */ select `outline_db`.`t1`.`id` AS `id`,`outline_db`.`t1`.`col1` AS `col1`,`outline_db`.`t1`.`col2` AS `col2` from `outline_db`.`t1` USE INDEX (`ind_1`) where ((`outline_db`.`t1`.`col2` = 'xpchild') and (`outline_db`.`t1`.`col1` = 2))
+delete from mysql.outline;
+commit;
+call dbms_outln.flush_outline();
+CALL dbms_outln.add_index_outline('outline_db', '', 1, 'USE INDEX', 'ind_2', '',
+"select * from t1 where t1.col1 =1 and t1.col2 ='xpchild'");
+EXPLAIN FORMAT = tree SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
+EXPLAIN
+-> Filter: (t1.col1 = 2)  (cost=0.35 rows=1)
+    -> Index lookup on t1 using ind_2 (col2='xpchild')  (cost=0.35 rows=1)
+
+EXPLAIN ANALYZE SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
+EXPLAIN
+-> Filter: (t1.col1 = 2)  (cost=0.35 rows=1) (actual time=0.033..0.035 rows=1 loops=1)
+    -> Index lookup on t1 using ind_2 (col2='xpchild')  (cost=0.35 rows=1) (actual time=0.031..0.033 rows=1 loops=1)
+
+EXPLAIN ANALYZE FORMAT=tree SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
+EXPLAIN
+-> Filter: (t1.col1 = 2)  (cost=0.35 rows=1) (actual time=0.024..0.027 rows=1 loops=1)
+    -> Index lookup on t1 using ind_2 (col2='xpchild')  (cost=0.35 rows=1) (actual time=0.023..0.025 rows=1 loops=1)
+
 delete from mysql.outline;
 commit;
 call dbms_outln.flush_outline();
diff --git a/mysql-test/suite/galaxy_sql/t/feature_outline.test b/mysql-test/suite/galaxy_sql/t/feature_outline.test
index a999d7dee12..c5f3fddf240 100644
--- a/mysql-test/suite/galaxy_sql/t/feature_outline.test
+++ b/mysql-test/suite/galaxy_sql/t/feature_outline.test
@@ -452,6 +452,15 @@ EXPLAIN FORMAT = TRADITIONAL SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpc
 EXPLAIN FORMAT = 'JsOn' SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
 EXPLAIN FORMAT = 'TrAdItIoNaL' SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
 
+delete from mysql.outline;
+commit;
+call dbms_outln.flush_outline();
+
+CALL dbms_outln.add_index_outline('outline_db', '', 1, 'USE INDEX', 'ind_2', '',
+                                "select * from t1 where t1.col1 =1 and t1.col2 ='xpchild'");
+EXPLAIN FORMAT = tree SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
+EXPLAIN ANALYZE SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
+EXPLAIN ANALYZE FORMAT=tree SELECT * FROM t1 WHERE t1.col1 =2 AND t1.col2 ='xpchild';
 
 delete from mysql.outline;
 commit;
diff --git a/sql/outline/outline_digest.cc b/sql/outline/outline_digest.cc
index 277763d47fd..dede6dbd4d1 100644
--- a/sql/outline/outline_digest.cc
+++ b/sql/outline/outline_digest.cc
@@ -31,18 +31,24 @@
 
 namespace im {
 
-#define EXPLAIN_TOKEN_SIZE 2
-#define EXPLAIN_FORMAT_MINIMAL_LENGTH 6
-#define IDENT_LENGTH_SIZE 2
-#define LEAST_TOKEN_SIZE 2
+#define SIZE_OF_A_TOKEN 2
+#define GET_TOKEN(p) (*(p) | (*(p+1) << 8))
+// The shortest valid real query should be "SELECT 1".
+#define EXPLAIN_QUERY_MININUM_LENGTH 4
+// param=value
+#define EXPLAIN_PARAM_MINIMAL_LENGTH 6
+#define ASSERT_LENGTH(len) \
+  assert(p + (len) + EXPLAIN_QUERY_MININUM_LENGTH <= end)
 
 /**
   Calculate the offset of the real query in the digest storage.
 
   The digesting process converts a SQL statement to normalized form by
   translating each token to a few digest bytes. This function calculates the
-  number of bytes that should be stripped to skip the digest bytes corresponding
-  to "EXPLAIN" or "EXPLAIN FORMAT = xxx".
+  number of bytes that should be stripped to skip the digest bytes until
+  the real query.
+
+  Assumed syntax: EXPLAIN [subcommand] [param=value] real_query
 
   @param[in]    digest_storage    Digest storage
 
@@ -51,64 +57,99 @@ namespace im {
 */
 uint calculate_strip_length_for_explain(
     const sql_digest_storage *digest_storage) {
-  uint strip_length = 0;
-
-  if (digest_storage && digest_storage->m_byte_count > EXPLAIN_TOKEN_SIZE) {
-    const unsigned char *src = &digest_storage->m_token_array[0];
-    uint tok = src[0] | (src[1] << 8);
-
-    if (tok == DESCRIBE) {
-      strip_length += 2;
-
-      uint expected_length = EXPLAIN_FORMAT_MINIMAL_LENGTH;
-      if (digest_storage->m_byte_count >=
-          EXPLAIN_TOKEN_SIZE + expected_length + LEAST_TOKEN_SIZE) {
-        /*
-          Make sure that there is enough size for "FORMAT = xxx" and at least
-          one more token.
-         */
-        uint tok_format = src[2] | (src[3] << 8);
-        uint tok_eq = src[4] | (src[5] << 8);
-        uint tok_type = src[6] | (src[7] << 8);
-
-        if (tok_format == FORMAT_SYM && tok_eq == EQ) {
-          /*
-            Note that the format type may also be specified as quoted:
-
-                EXPLAIN FORMAT='TrAdItIoNaL' SELECT 1
-
-            The quoted type is reduced as TOK_GENERIC_VALUE in digest_storage.
-            However, the input token stream is intact and still recognized
-            by the parser.
-           */
-          if (tok_type == JSON_SYM || tok_type == TOK_GENERIC_VALUE)
-            strip_length += expected_length;
-          else if (tok_type == TOK_IDENT) {
-            expected_length += IDENT_LENGTH_SIZE;
-            if (digest_storage->m_byte_count >=
-                EXPLAIN_TOKEN_SIZE + expected_length + LEAST_TOKEN_SIZE) {
-              uint id_len = src[8] | (src[9] << 8);
-              expected_length += id_len;
-              if (digest_storage->m_byte_count >=
-                  EXPLAIN_TOKEN_SIZE + expected_length + LEAST_TOKEN_SIZE) {
-                strip_length += expected_length;
-              } else {
-                // Incomplete identifier payload
-                assert(0);
-              }
-            } else {
-              // Missing identifier length
-              assert(0);
+  const unsigned char *p = digest_storage->m_token_array;
+  const unsigned char *end = p + digest_storage->m_token_array_length;
+
+  // EXPLAIN
+  if (unlikely(!p || (p + SIZE_OF_A_TOKEN) > end)) return 0;
+
+  uint tok = GET_TOKEN(p);
+  if (tok != DESCRIBE) {
+    // OK, non-EXPLAIN
+    return 0;
+  }
+  p += SIZE_OF_A_TOKEN;
+
+  // [subcommand] [param=value] real_query
+  ASSERT_LENGTH(0);
+
+  tok = GET_TOKEN(p);
+  switch (tok) {
+    case FORMAT_SYM: {
+      /*
+        FORMAT = { json | traditional | tree | <quoted> } real_query
+        Note that the format type may also be specified as quoted:
+
+            EXPLAIN FORMAT='TrAdItIoNaL' SELECT 1
+
+        The quoted type is reduced as TOK_GENERIC_VALUE in digest_storage.
+        However, the input token stream is intact and still recognized
+        by the parser.
+       */
+      ASSERT_LENGTH(EXPLAIN_PARAM_MINIMAL_LENGTH);
+      assert(GET_TOKEN(p + SIZE_OF_A_TOKEN) == EQ);
+      switch (GET_TOKEN(p + SIZE_OF_A_TOKEN + SIZE_OF_A_TOKEN)) {
+        case JSON_SYM:  // fallthrough
+        case TOK_GENERIC_VALUE: {
+          p += EXPLAIN_PARAM_MINIMAL_LENGTH;
+          break;
+        }
+        case TOK_IDENT: {
+          p += EXPLAIN_PARAM_MINIMAL_LENGTH;
+          uint len = GET_TOKEN(p);
+          ASSERT_LENGTH(len);
+          p += SIZE_OF_A_TOKEN + len;
+          break;
+        }
+        default: {
+          assert(false);
+          return 0;
+        }
+      }
+      break;
+    }
+    case ANALYZE_SYM:
+      p += SIZE_OF_A_TOKEN;
+      tok = GET_TOKEN(p);
+      switch (tok) {
+        case FORMAT_SYM: {
+          // FORMAT = { tree | <quoted> }
+          ASSERT_LENGTH(EXPLAIN_PARAM_MINIMAL_LENGTH);
+          assert(GET_TOKEN(p + SIZE_OF_A_TOKEN) == EQ);
+          switch (GET_TOKEN(p + SIZE_OF_A_TOKEN + SIZE_OF_A_TOKEN)) {
+            case TOK_GENERIC_VALUE: {
+              p += EXPLAIN_PARAM_MINIMAL_LENGTH;
+              break;
+            }
+            case TOK_IDENT: {
+              p += EXPLAIN_PARAM_MINIMAL_LENGTH;
+              uint len = GET_TOKEN(p);
+              ASSERT_LENGTH(len);
+              p += SIZE_OF_A_TOKEN + len;
+              break;
+            }
+            default: {
+              assert(false);
+              return 0;
             }
-          } else {
-            // Unexpected EXPLAIN format type token
-            assert(0);
           }
-        }  // FORMAT = { json | traditional | tree }
-      }
-    }  // DESCRIBE
+          break;
+        }
+        default:
+          // no parameter
+          assert(GET_TOKEN(p + SIZE_OF_A_TOKEN) != EQ);
+          break;
+      }
+      break;
+    default:
+      // no parameter
+      assert(GET_TOKEN(p + SIZE_OF_A_TOKEN) != EQ);
+      break;
   }
-  return strip_length;
+
+  // The shortest valid statment should be "SELECT 1".
+  assert(p + EXPLAIN_QUERY_MININUM_LENGTH <= end);
+  return (p - digest_storage->m_token_array);
 }
 
 /**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant