MDEV-30395 Wrong result with semijoin and Federated as outer table

The problem was that federated engine does not support comparable rowids
which was not taken into account by semijoin code.

Fixed by checking that we don't use semijoin with tables that does not
support comparable rowids.

Other things:
- Fixed some typos in the code comments

mysql-test/suite/federated/federatedx.result

@@ -2325,6 +2325,38 @@ DROP TABLE federated.t1;
 connection slave;
 DROP TABLE federated.t1;
 connection default;
+#
+# MDEV-30395 Wrong result with semijoin and Federated as outer table
+#
+create server s foreign data wrapper mysql options (host "127.0.0.1", database "test", user "root", port MASTER_PORT);
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (3),(2),(3);
+CREATE TABLE t2 (pk INT PRIMARY KEY);
+INSERT INTO t2 VALUES (1),(2),(3),(4);
+set @save_optimizer_switch=@@optimizer_switch;
+set optimizer_switch="materialization=off";
+CREATE TABLE t2_fed ENGINE=FEDERATED CONNECTION='s/t2';
+explain SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t2_fed	ALL	NULL	NULL	NULL	NULL	4	Using where
+2	DEPENDENT SUBQUERY	t1	ALL	NULL	NULL	NULL	NULL	3	Using where
+SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+pk
+2
+3
+SET optimizer_switch='semijoin=off';
+explain SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t2_fed	ALL	NULL	NULL	NULL	NULL	4	Using where
+2	DEPENDENT SUBQUERY	t1	ALL	NULL	NULL	NULL	NULL	3	Using where
+SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+pk
+2
+3
+DROP TABLE t2_fed, t1, t2;
+set @@optimizer_switch=@save_optimizer_switch;
+DROP SERVER s;
+# End of 10.5 tests
 connection master;
 DROP TABLE IF EXISTS federated.t1;
 DROP DATABASE IF EXISTS federated;

mysql-test/suite/federated/federatedx.test

@@ -2060,4 +2060,34 @@ connection slave;
 DROP TABLE federated.t1;
 connection default;
 
+--echo #
+--echo # MDEV-30395 Wrong result with semijoin and Federated as outer table
+--echo #
+
+
+--replace_result $MASTER_MYPORT MASTER_PORT
+eval create server s foreign data wrapper mysql options (host "127.0.0.1", database "test", user "root", port $MASTER_MYPORT);
+
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (3),(2),(3);
+CREATE TABLE t2 (pk INT PRIMARY KEY);
+INSERT INTO t2 VALUES (1),(2),(3),(4);
+
+set @save_optimizer_switch=@@optimizer_switch;
+set optimizer_switch="materialization=off";
+
+CREATE TABLE t2_fed ENGINE=FEDERATED CONNECTION='s/t2';
+explain SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+SET optimizer_switch='semijoin=off';
+explain SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+SELECT * FROM t2_fed WHERE pk IN ( SELECT a FROM t1 );
+
+DROP TABLE t2_fed, t1, t2;
+set @@optimizer_switch=@save_optimizer_switch;
+
+DROP SERVER s;
+
+--echo # End of 10.5 tests
+
 source include/federated_cleanup.inc;

sql/handler.h

@@ -356,9 +356,9 @@ enum chf_create_flags {
   Rowid's are not comparable. This is set if the rowid is unique to the
   current open handler, like it is with federated where the rowid is a
   pointer to a local result set buffer. The effect of having this set is
-  that the optimizer will not consirer the following optimizations for
+  that the optimizer will not consider the following optimizations for
   the table:
-  ror scans or filtering
+  ror scans, filtering or duplicate weedout
 */
 #define HA_NON_COMPARABLE_ROWID (1ULL << 60)
 

sql/opt_subselect.cc

@@ -664,6 +664,17 @@ int check_and_do_in_subquery_rewrites(JOIN *join)
         DBUG_RETURN(-1);
       }
     }
+    /* Check if any table is not supporting comparable rowids */
+    {
+      List_iterator_fast<TABLE_LIST> li(select_lex->outer_select()->leaf_tables);
+      TABLE_LIST *tbl;
+      while ((tbl = li++))
+      {
+        TABLE *table= tbl->table;
+        if (table && table->file->ha_table_flags() & HA_NON_COMPARABLE_ROWID)
+          join->not_usable_rowid_map|= table->map;
+      }
+    }
 
     DBUG_PRINT("info", ("Checking if subq can be converted to semi-join"));
     /*
@@ -685,6 +696,9 @@ int check_and_do_in_subquery_rewrites(JOIN *join)
         11. It is first optimisation (the subquery could be moved from ON
             clause during first optimisation and then be considered for SJ
             on the second when it is too late)
+        12. All tables supports comparable rowids.
+            This is needed for DuplicateWeedout strategy to work (which
+            is the catch-all semi-join strategy so it must be applicable).
     */
     if (optimizer_flag(thd, OPTIMIZER_SWITCH_SEMIJOIN) &&
         in_subs &&                                                    // 1
@@ -699,7 +713,8 @@ int check_and_do_in_subquery_rewrites(JOIN *join)
         !((join->select_options |                                     // 10
            select_lex->outer_select()->join->select_options)          // 10
           & SELECT_STRAIGHT_JOIN) &&                                  // 10
-        select_lex->first_cond_optimization)                          // 11
+        select_lex->first_cond_optimization &&                        // 11
+        join->not_usable_rowid_map == 0)                              // 12
     {
       DBUG_PRINT("info", ("Subquery is semi-join conversion candidate"));
 
@@ -3544,6 +3559,9 @@ bool Duplicate_weedout_picker::check_qep(JOIN *join,
       }
       else
       {
+        /* Ensure that table supports comparable rowids */
+        DBUG_ASSERT(!(p->table->table->file->ha_table_flags() & HA_NON_COMPARABLE_ROWID));
+
         sj_outer_fanout= COST_MULT(sj_outer_fanout, p->records_read);
         temptable_rec_size += p->table->table->file->ref_length;
       }

sql/sql_select.cc

@@ -2309,7 +2309,7 @@ JOIN::optimize_inner()
   /*
     We have to remove constants and duplicates from group_list before
     calling make_join_statistics() as this may call get_best_group_min_max()
-    which needs a simplfied group_list.
+    which needs a simplified group_list.
   */
   if (group_list && table_count == 1)
   {

sql/sql_select.h

@@ -1238,6 +1238,8 @@ public:
   table_map outer_join;
   /* Bitmap of tables used in the select list items */
   table_map select_list_used_tables;
+  /* Tables that has HA_NON_COMPARABLE_ROWID (does not support rowid) set */
+  table_map not_usable_rowid_map;
   ha_rows  send_records,found_records,join_examined_rows;
 
   /*
@@ -1550,7 +1552,7 @@ public:
     table_count= 0;
     top_join_tab_count= 0;
     const_tables= 0;
-    const_table_map= found_const_table_map= 0;
+    const_table_map= found_const_table_map= not_usable_rowid_map= 0;
     aggr_tables= 0;
     eliminated_tables= 0;
     join_list= 0;