[MDEV-26111] Spider handle ">=" as "=" in some cases Created: 2021-07-08  Updated: 2023-05-23

Status: Stalled
Project: MariaDB Server
Component/s: Storage Engine - Spider
Affects Version/s: 10.2
Fix Version/s: 10.2

Type: Bug Priority: Major
Reporter: Nayuta Yanagisawa (Inactive) Assignee: Yuchen Pei
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Through the investigation for MDEV-25985, I found another bug of the Spider's query rewiring process.

How to reproduce:

source ../storage/spider/scripts/install_spider.sql;
GRANT ALL PRIVILEGES ON *.* TO 'spinne'@'127.0.0.1'  IDENTIFIED BY 'password';
 
CREATE SERVER IF NOT EXISTS data1
FOREIGN DATA WRAPPER mysql
OPTIONS(
HOST '127.0.0.1',
DATABASE 'test',
USER 'spinne',
PORT 16000,
PASSWORD 'password'
);
 
DROP TABLE IF EXISTS `test`.`td`;
 
CREATE TABLE `test`.`td` (
`a` int,
`b` int,
`c` int,
PRIMARY KEY (`a`),
KEY (`b`,`c`)
) ENGINE=InnoDB ;
 
DROP TABLE IF EXISTS `test`.`ts`;
 
CREATE TABLE `test`.`ts` (
`a` int,
`b` int,
`c` int,
PRIMARY KEY (`a`),
KEY (`b`,`c`)
) ENGINE=SPIDER COMMENT='table "td"'
PARTITION BY LIST COLUMNS(`a`)
(PARTITION `ptdef` DEFAULT COMMENT = 'srv "data1"' ENGINE = SPIDER);
 
INSERT INTO test.td VALUES (1,1,1), (2,2,1);

MariaDB [(none)]> SELECT * FROM test.td B WHERE B.c > 0 AND B.b >= 1;
+---+------+------+
| a | b    | c    |
+---+------+------+
| 1 |    1 |    1 |
| 2 |    2 |    1 |
+---+------+------+
2 rows in set (0.00 sec)
 
MariaDB [(none)]>  SELECT * FROM test.ts B WHERE B.c > 0 AND B.b >= 1;
+---+------+------+
| a | b    | c    |
+---+------+------+
| 1 |    1 |    1 |
+---+------+------+
1 row in set (0.05 sec)

The query, executed on the data node, corresponding to one executed on the Spider node is the following:

 select `a`,`b`,`c` from `test`.`td` where `b` = 1 and `c` > 0 and ((`c` > 0) and (`b` >= 1))



 Comments   
Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-26 ]

I cannot reproduce the bug on 10.3 HEAD (f52d39369a6e3f1a9ce89189180d142e54299862) with the test case attached to the issue description. Of course, there is the possibility that we could reproduce a similar bug with much more complex queries.

MariaDB [(none)]> SELECT * FROM test.td B WHERE B.c > 0 AND B.b >= 1;
SELECT * FROM test.ts B WHERE B.c > 0 AND B.b >= 1;
+---+------+------+
| a | b    | c    |
+---+------+------+
| 1 |    1 |    1 |
| 2 |    2 |    1 |
+---+------+------+
2 rows in set (0.001 sec)
 
MariaDB [(none)]> SELECT * FROM test.ts B WHERE B.c > 0 AND B.b >= 1;
+---+------+------+
| a | b    | c    |
+---+------+------+
| 1 |    1 |    1 |
| 2 |    2 |    1 |
+---+------+------+
2 rows in set (0.028 sec)

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-26 ]

Again, I think that the above does not mean that there is no logic error in 10.3 or beyond. This is speculation, but I guess that the errors are just not showing up because of changes of the execution paths.

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

The issue looks somewhat similar to MDEV-25985. In fact, the root cause of the present bug is in spider_db_append_key_where_internal() too. In the function, the Spider SE wrongly set key_eq = TRUE for the first column of a composite index in some cases. This results in the wrong conversion of b => 1 to b = 1.

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

A possible fix would be the following. All the tests on 10.2 pass with the fix, but I will look into it further to ensure the fix doesn't affect the other part of the code.

diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc
index df6c0ceb5f5..34f32a5d873 100644
--- a/storage/spider/spd_db_conn.cc
+++ b/storage/spider/spd_db_conn.cc
@@ -1734,10 +1734,7 @@ int spider_db_append_key_where_internal(
       }
     } else {
       DBUG_PRINT("info", ("spider tgt_key_part_map=%lu", tgt_key_part_map));
-      if (tgt_key_part_map > 1)
-        key_eq = TRUE;
-      else
-        key_eq = FALSE;
+      key_eq = FALSE;
     }
     if (
       (key_eq && use_key == start_key) ||

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

All the tests on 10.6 HEAD also pass with the above fix.

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

use_both == false (i.e., end_key == NULL) doesn't implies the original search condition is an equality. In fact, if we specifies the search condition B.c > 0 AND B.b >= 1, then we get end_key == NULL. It seems to be wrong to set key_eq = TRUE when use_both && tgt_key_part_map > 1

Thread 27 "mysqld" hit Breakpoint 1, ha_spider::read_range_first (this=0x5605cfb51943 <code_state+171>, start_key=0x7fdbe0eeaf60, end_key=0x7fdb84000b60, eq_range=219, sorted=208)
    at /home/vagrant/repo/mariadb-server/storage/spider/ha_spider.cc:4278
4278    ) {
(gdb) p *start_key
$2 = {key = 0x7fdb8406f108 "", length = 10, keypart_map = 3, flag = HA_READ_AFTER_KEY}
(gdb) p end_key
$3 = (const key_range *) 0x0

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

By the above fix, we get a correct query for a data node:

 info: spider query=select `a`,`b`,`c` from `auto_test_remote`.`t1` where `b` >= 1 and `c` > 0 and ((`b` >= 1) and (`c` > 0))

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

serg Please review: https://github.com/MariaDB/server/commit/ea162a60324481c16aeaed838093abddc5a22061

Comment by Nayuta Yanagisawa (Inactive) [ 2021-07-28 ]

serg I edited the commit message. Please review the following rather than the above: https://github.com/MariaDB/server/commit/3efc826bff5ad40869da5fb6367b8b250182ad04

Comment by Nayuta Yanagisawa (Inactive) [ 2021-08-13 ]

serg I may miss something. Let me withdraw the patch.

Comment by Nayuta Yanagisawa (Inactive) [ 2021-09-01 ]

The bug doesn't reproduce on 10.3 because it skips the part of spider_db_append_key_where_internal() which prints the wrong WHERE condition by jumping to the end label.

  if (!start_key_part_map && !end_key_part_map)
  {
    ...
  } else if (use_both && (!start_key_part_map || !end_key_part_map))
  {
    result_list->key_order = 0;
    goto end;  /* HERE! */
  }
  ..

Generated at Thu Feb 08 09:42:49 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.