fix: MDEV-40846 DuckDB handles functionality that is based on invisible columns, e.g. WITH SYSTEM VERSIONING.

storage/duckdb/docs/architecture.md

@@ -57,8 +57,9 @@ Primary path for analytical queries. MariaDB hands the **entire SELECT** to Duck
 ```
 MariaDB parser
   → optimizer calls hton->create_select / hton->create_unit
-    → can_pushdown_to_duckdb(): at least one DuckDB table?
-      → creates ha_duckdb_select_handler
+    → factory eligibility checks in ha_duckdb_pushdown.cc
+      → requires at least one DuckDB table and supported query/table metadata
+        → creates ha_duckdb_select_handler
         → init_scan():
            1. Takes original SQL text from THD::query()
            2. Rewrites MariaDB-specific syntax for DuckDB:
@@ -74,11 +75,35 @@ MariaDB parser
         → end_scan(): releases result
 ```
 
-#### Single-SELECT pushdown eligibility
+#### Pushdown eligibility
 
-The single-SELECT handler declines pushdown when MariaDB marks the top-level
-`SELECT_LEX` with `UNCACHEABLE_SIDEEFFECT`. The regular handler path (`rnd_*`)
-is used instead. MariaDB sets this flag for:
+Both the single-SELECT and whole-unit factories in `ha_duckdb_pushdown.cc`
+decline pushdown for:
+
+- an explicit `FOR SYSTEM_TIME` clause, because DuckDB does not implement
+  MariaDB system-versioning syntax;
+- an explicit reference to an invisible column, including references in the
+  select list, `WHERE`, `HAVING`, `GROUP BY`, `ORDER BY`, join conditions, and
+  window `PARTITION BY` / `ORDER BY` specifications;
+- an external table where a visible column follows an invisible column, because
+  `_mdb_scan` can expose only a visible prefix while preserving MariaDB field
+  indexes;
+- any invisible column on an external table when
+  `duckdb_cross_engine_ryow=ON`; the direct handler scan bypasses the synthetic
+  MariaDB SELECT that handles hidden metadata and implicit conditions.
+
+With `duckdb_cross_engine_ryow=OFF`, trailing invisible columns are supported:
+`_mdb_scan` exposes only the visible prefix, while MariaDB applies implicit
+system-versioning conditions inside the synthetic external-table SELECT.
+Automatically generated system-versioning field references do not count as
+explicit references.
+
+Returning no select handler makes MariaDB use its regular execution path; that
+fallback can itself reject a mixed-engine plan if it requests an unsupported
+DuckDB handler operation.
+
+The single-SELECT factory additionally declines pushdown when MariaDB marks the
+top-level `SELECT_LEX` with `UNCACHEABLE_SIDEEFFECT`. MariaDB sets this flag for:
 
 - reads and assignments of user variables (`@var`, `@var := expr`);
 - reads of session or global system variables (`@@session.var`, `@@global.var`);
@@ -115,10 +140,14 @@ The fiber runs on the same OS thread as DuckDB. TLS (`current_thd`, `THR_KEY_mys
 
 #### Pushdown modes: `duckdb_cross_engine_ryow`
 
-The session variable `duckdb_cross_engine_ryow` (default `OFF`) selects, per
-query, how external (non-DuckDB) tables are read. The flag is captured once in
-`init_scan()` (`register_cross_engine_ryow()`), and the replacement scan then
-redirects to one of two table functions.
+For an eligible query, the session variable `duckdb_cross_engine_ryow`
+(default `OFF`) selects how external (non-DuckDB) tables are read. The flag is
+captured by `ha_duckdb_select_handler::init_scan()` in
+`ha_duckdb_pushdown.cc`, which calls `register_cross_engine_ryow()`; the
+replacement scan then redirects to one of two table functions. Eligibility is
+checked first: with RYOW enabled, an external table containing any invisible
+column makes the factory decline pushdown rather than select the direct table
+function.
 
 | Aspect | `duckdb_cross_engine_ryow = OFF` (default) | `duckdb_cross_engine_ryow = ON` |
 |---|---|---|

storage/duckdb/ha_duckdb_pushdown.cc

@@ -21,6 +21,7 @@
 #include <my_global.h>
 #include "sql_class.h"
 #include "sql_select.h"
+#include "sql_window.h"
 #include "log.h"
 
 #undef UNKNOWN
@@ -35,6 +36,115 @@
 
 extern handlerton *duckdb_hton;
 
+class Invisible_field_enumerator : public Field_enumerator
+{
+public:
+  explicit Invisible_field_enumerator(THD *thd) : m_thd(thd) {}
+
+  void visit_field(Item_field *item) override
+  {
+    if (m_found || !item->field || item->field->invisible == VISIBLE ||
+        is_implicit_versioning_field(item))
+      return;
+    m_found= true;
+  }
+
+  bool found() const { return m_found; }
+
+private:
+  bool is_implicit_versioning_field(Item_field *item) const
+  {
+    for (TABLE_LIST *tbl= m_thd->lex->query_tables; tbl;
+         tbl= tbl->next_global)
+    {
+      if (item == tbl->vers_conditions.field_start ||
+          item == tbl->vers_conditions.field_end)
+        return true;
+    }
+    return false;
+  }
+
+  THD *m_thd;
+  bool m_found= false;
+};
+
+static void inspect_item_for_invisible_fields(
+    Item *item, Invisible_field_enumerator &enumerator)
+{
+  if (item && !enumerator.found())
+    item->walk(&Item::enumerate_field_refs_processor, false, &enumerator);
+}
+
+static bool query_references_invisible_fields(THD *thd)
+{
+  Invisible_field_enumerator enumerator(thd);
+
+  for (SELECT_LEX *sl= thd->lex->all_selects_list; sl && !enumerator.found();
+       sl= sl->next_select_in_list())
+  {
+    List_iterator_fast<Item> items(sl->item_list);
+    Item *item;
+    while ((item= items++))
+      inspect_item_for_invisible_fields(item, enumerator);
+
+    inspect_item_for_invisible_fields(sl->where, enumerator);
+    inspect_item_for_invisible_fields(sl->having, enumerator);
+
+    for (ORDER *order= sl->group_list.first; order; order= order->next)
+      inspect_item_for_invisible_fields(*order->item, enumerator);
+    for (ORDER *order= sl->order_list.first; order; order= order->next)
+      inspect_item_for_invisible_fields(*order->item, enumerator);
+
+    List_iterator_fast<Window_spec> windows(sl->window_specs);
+    Window_spec *window;
+    while ((window= windows++))
+    {
+      for (ORDER *order= window->partition_list->first; order;
+           order= order->next)
+        inspect_item_for_invisible_fields(*order->item, enumerator);
+      for (ORDER *order= window->order_list->first; order;
+           order= order->next)
+        inspect_item_for_invisible_fields(*order->item, enumerator);
+    }
+
+    for (TABLE_LIST *tbl= sl->get_table_list(); tbl; tbl= tbl->next_local)
+      inspect_item_for_invisible_fields(tbl->on_expr, enumerator);
+  }
+
+  return enumerator.found();
+}
+
+static bool query_has_explicit_system_time(THD *thd)
+{
+  for (TABLE_LIST *tbl= thd->lex->query_tables; tbl; tbl= tbl->next_global)
+    if (tbl->vers_conditions.was_set())
+      return true;
+  return false;
+}
+
+static bool has_supported_field_layout(TABLE *table,
+                                       bool allow_trailing_invisible)
+{
+  bool found_invisible= false;
+
+  for (Field **field= table->field; *field; field++)
+  {
+    if ((*field)->invisible == VISIBLE)
+    {
+      if (found_invisible)
+        return false;
+    }
+    else
+    {
+      if (!allow_trailing_invisible)
+        return false;
+      found_invisible= true;
+    }
+  }
+
+  return true;
+}
+
 /**
   Check whether a SELECT_LEX can be pushed down to DuckDB.
 
@@ -45,7 +155,8 @@ extern handlerton *duckdb_hton;
 
 static bool can_pushdown_to_duckdb(SELECT_LEX *sel_lex,
                                    std::vector<std::string> &external_tables,
-                                   bool &has_duckdb_table)
+                                   bool &has_duckdb_table,
+                                   bool allow_trailing_invisible)
 {
   for (TABLE_LIST *tbl= sel_lex->get_table_list(); tbl; tbl= tbl->next_global)
   {
@@ -55,7 +166,12 @@ static bool can_pushdown_to_duckdb(SELECT_LEX *sel_lex,
     if (tbl->table->file->ht == duckdb_hton)
       has_duckdb_table= true;
     else
+    {
+      if (!has_supported_field_layout(tbl->table,
+                                      allow_trailing_invisible))
+        return false;
       external_tables.emplace_back(tbl->table_name.str);
+    }
   }
 
   return has_duckdb_table;
@@ -69,7 +185,8 @@ static bool can_pushdown_to_duckdb(SELECT_LEX *sel_lex,
 static bool
 can_pushdown_unit_to_duckdb(SELECT_LEX_UNIT *unit,
                             std::vector<std::string> &external_tables,
-                            bool &has_duckdb_table)
+                            bool &has_duckdb_table,
+                            bool allow_trailing_invisible)
 {
   for (SELECT_LEX *sl= unit->first_select(); sl; sl= sl->next_select())
   {
@@ -81,7 +198,12 @@ can_pushdown_unit_to_duckdb(SELECT_LEX_UNIT *unit,
       if (tbl->table->file->ht == duckdb_hton)
         has_duckdb_table= true;
       else
+      {
+        if (!has_supported_field_layout(tbl->table,
+                                        allow_trailing_invisible))
+          return false;
         external_tables.emplace_back(tbl->table_name.str);
+      }
     }
   }
 
@@ -212,7 +334,9 @@ select_handler *create_duckdb_select_handler(THD *thd, SELECT_LEX *sel_lex,
     return nullptr;
 
   if (!sel_lex || has_duckdb_insert_target(thd) ||
-      has_unsupported_insert_select_clauses(thd))
+      has_unsupported_insert_select_clauses(thd) ||
+      query_has_explicit_system_time(thd) ||
+      query_references_invisible_fields(thd))
     return nullptr;
 
   std::string query;
@@ -222,7 +346,8 @@ select_handler *create_duckdb_select_handler(THD *thd, SELECT_LEX *sel_lex,
   std::vector<std::string> external_tables;
   bool has_duckdb_table= false;
 
-  if (!can_pushdown_to_duckdb(sel_lex, external_tables, has_duckdb_table))
+  if (!can_pushdown_to_duckdb(sel_lex, external_tables, has_duckdb_table,
+                              !myduck::get_thd_cross_engine_ryow(thd)))
     return nullptr;
 
   /* At least one DuckDB table must participate */
@@ -252,7 +377,9 @@ select_handler *create_duckdb_unit_handler(THD *thd, SELECT_LEX_UNIT *sel_unit)
   if ((thd->lex->sql_command != SQLCOM_SELECT &&
        thd->lex->sql_command != SQLCOM_INSERT_SELECT) ||
       has_duckdb_insert_target(thd) ||
-      has_unsupported_insert_select_clauses(thd))
+      has_unsupported_insert_select_clauses(thd) ||
+      query_has_explicit_system_time(thd) ||
+      query_references_invisible_fields(thd))
     return nullptr;
 
   if (thd->stmt_arena && thd->stmt_arena->is_stmt_prepare())
@@ -268,8 +395,9 @@ select_handler *create_duckdb_unit_handler(THD *thd, SELECT_LEX_UNIT *sel_unit)
   std::vector<std::string> external_tables;
   bool has_duckdb_table= false;
 
-  if (!can_pushdown_unit_to_duckdb(sel_unit, external_tables,
-                                   has_duckdb_table))
+  if (!can_pushdown_unit_to_duckdb(
+          sel_unit, external_tables, has_duckdb_table,
+          !myduck::get_thd_cross_engine_ryow(thd)))
     return nullptr;
 
   if (!has_duckdb_table)
@@ -888,9 +1016,15 @@ int ha_duckdb_select_handler::next_row()
   for (Field **f= table->field; *f; f++)
     field_count++;
 
-  size_t ncols= (col_count < field_count) ? col_count : field_count;
+  if (col_count != field_count)
+  {
+    my_error(ER_GET_ERRMSG, MYF(0), HA_ERR_INTERNAL_ERROR,
+             "DuckDB result column count does not match MariaDB metadata",
+             "DuckDB");
+    DBUG_RETURN(HA_ERR_INTERNAL_ERROR);
+  }
 
-  for (size_t col_idx= 0; col_idx < ncols; col_idx++)
+  for (size_t col_idx= 0; col_idx < col_count; col_idx++)
   {
     duckdb::Value value= current_chunk->GetValue(col_idx, current_row_index);
     Field *field= table->field[col_idx];

storage/duckdb/mysql-test/duckdb/r/cross_engine_join.result

@@ -117,6 +117,65 @@ id	dval	ival
 1	a	NULL
 2	NULL	b
 
+# (9) MDEV-40846: trailing system-versioning columns stay hidden
+
+SET SESSION default_storage_engine=DuckDB;
+CREATE TABLE t_versioned (
+c1 INT KEY,
+c2 DEC(1,0) NOT NULL,
+c3 SET('a','b','c','1'),
+c4 INT(1) UNSIGNED NOT NULL,
+KEY(c2)
+) ENGINE=MyISAM WITH SYSTEM VERSIONING;
+INSERT INTO t_versioned (c1, c2, c3, c4) VALUES (1, 0, 1, 0);
+CREATE TABLE t_default (c0 VARCHAR(1) BINARY, c1 FLOAT KEY, c2 FLOAT);
+WITH a AS (SELECT * FROM t_versioned),
+b AS (SELECT * FROM t_versioned),
+c AS (SELECT * FROM t_default)
+SELECT * FROM a JOIN b ON a.c2=b.c2 LEFT JOIN c ON b.c2=c.c2;
+c1	c2	c3	c4	c1	c2	c3	c4	c0	c1	c2
+1	0	a	0	1	0	a	0	NULL	NULL	NULL
+SET SESSION default_storage_engine=DEFAULT;
+
+# (10) Explicit trailing invisible column uses MariaDB fallback
+
+CREATE TABLE t_trailing_invisible (
+id INT PRIMARY KEY,
+hidden INT INVISIBLE
+) ENGINE=InnoDB;
+INSERT INTO t_trailing_invisible (id, hidden) VALUES (1, 10);
+SELECT d.id, i.hidden
+FROM t_duck d JOIN t_trailing_invisible i ON d.id=i.id;
+ERROR HY000: Storage engine DUCKDB of the table `cross_engine_join`.`t_duck` doesn't have this option
+
+# (11) Non-trailing invisible column uses MariaDB fallback
+
+CREATE TABLE t_nontrailing_invisible (
+id INT PRIMARY KEY,
+hidden INT INVISIBLE,
+val INT
+) ENGINE=InnoDB;
+INSERT INTO t_nontrailing_invisible (id, hidden, val) VALUES (1, 10, 100);
+SELECT d.id, i.val
+FROM t_duck d JOIN t_nontrailing_invisible i ON d.id=i.id;
+ERROR HY000: Storage engine DUCKDB of the table `cross_engine_join`.`t_duck` doesn't have this option
+
+# (12) Explicit FOR SYSTEM_TIME uses MariaDB fallback
+
+SELECT d.id, i.c2
+FROM t_duck d
+JOIN t_versioned FOR SYSTEM_TIME ALL AS i ON d.id=i.c1;
+ERROR HY000: Storage engine DUCKDB of the table `cross_engine_join`.`t_duck` doesn't have this option
+
+# (13) Invisible columns in window specifications use MariaDB fallback
+
+SELECT ROW_NUMBER() OVER (PARTITION BY i.hidden)
+FROM t_duck d JOIN t_trailing_invisible i ON d.id=i.id;
+ERROR HY000: Storage engine DUCKDB of the table `cross_engine_join`.`t_duck` doesn't have this option
+SELECT ROW_NUMBER() OVER (ORDER BY i.hidden)
+FROM t_duck d JOIN t_trailing_invisible i ON d.id=i.id;
+ERROR HY000: Storage engine DUCKDB of the table `cross_engine_join`.`t_duck` doesn't have this option
+
 # Cleanup
 
 DROP TABLE t_duck;
@@ -125,4 +184,8 @@ DROP TABLE t_duck_types;
 DROP TABLE t_inno_types;
 DROP TABLE t_duck_null;
 DROP TABLE t_inno_null;
+DROP TABLE t_versioned;
+DROP TABLE t_default;
+DROP TABLE t_trailing_invisible;
+DROP TABLE t_nontrailing_invisible;
 DROP DATABASE cross_engine_join;