MDEV-25202: JSON_TABLE: Early table reference leads to unexpected result set

Address review input: switch Name_resolution_context::ignored_tables from
table_map to a list of TABLE_LIST objects. The rationale is that table
bits may be changed due to query rewrites, etc, which may potentially
require updating ignored_tables.

mysql-test/suite/json/t/json_table.test

@@ -682,6 +682,19 @@ FROM
     ON tt.b = jt.o
  ON t1.a = tt.b;
 
+prepare s from
+"SELECT *
+FROM
+  t1 RIGHT JOIN
+    v2 AS tt
+    LEFT JOIN
+    JSON_TABLE(CONCAT(tt.c,''), '$' COLUMNS(o FOR ORDINALITY)) AS jt
+    ON tt.b = jt.o
+ ON t1.a = tt.b";
+execute s;
+execute s;
+
+
 DROP VIEW v2;
 DROP TABLE t1, t2;
 

sql/item.cc

@@ -10713,3 +10713,18 @@ bool Item::cleanup_excluding_immutables_processor (void *arg)
     return false;
   }
 }
+
+
+bool ignored_list_includes_table(ignored_tables_list_t list, TABLE_LIST *tbl)
+{
+  if (!list)
+    return false;
+  List_iterator<TABLE_LIST> it(*list);
+  TABLE_LIST *list_tbl;
+  while ((list_tbl = it++))
+  {
+    if (list_tbl == tbl)
+      return true;
+  }
+  return false;
+}

sql/item.h

@@ -162,6 +162,9 @@ void dummy_error_processor(THD *thd, void *data);
 
 void view_error_processor(THD *thd, void *data);
 
+typedef List<TABLE_LIST>* ignored_tables_list_t;
+bool ignored_list_includes_table(ignored_tables_list_t list, TABLE_LIST *tbl);
+
 /*
   Instances of Name_resolution_context store the information necessary for
   name resolution of Items and other context analysis of a query made in
@@ -236,7 +239,7 @@ struct Name_resolution_context: Sql_alloc
     Bitmap of tables that should be ignored when doing name resolution.
     Normally it is {0}. Non-zero values are used by table functions.
   */
-  table_map ignored_tables;
+  ignored_tables_list_t ignored_tables;
 
   /*
     Security context of this name resolution context. It's used for views
@@ -247,7 +250,7 @@ struct Name_resolution_context: Sql_alloc
   Name_resolution_context()
     :outer_context(0), table_list(0), select_lex(0),
     error_processor_data(0),
-    ignored_tables(0),
+    ignored_tables(NULL),
     security_ctx(0)
     {}
 

sql/json_table.cc

@@ -44,9 +44,14 @@ static table_function_handlerton table_function_hton;
 
 /*
   @brief
-    Collect a bitmap of tables that a given table function cannot have
+    Collect a set of tables that a given table function cannot have
     references to.
 
+  @param
+     table_func         The table function we are connecting info for
+     join_list          The nested join to be processed
+     disallowed_tables  Collect the tables here.
+
   @detail
     According to the SQL standard, a table function can refer to any table
     that's "preceding" it in the FROM clause.
@@ -80,16 +85,16 @@ static table_function_handlerton table_function_hton;
     we are ok with operating on the tables "in the left join order".
 
   @return
-    TRUE  - enumeration has found the Table Function instance. The bitmap is
-            ready.
-    FALSE - Otherwise
-
+     0 - Continue
+     1 - Finish the process, success
+    -1 - Finish the process, failure
 */
 
 static
-bool get_disallowed_table_deps_for_list(table_map table_func_bit,
-                                        List<TABLE_LIST> *join_list,
-                                        table_map *disallowed_tables)
+int get_disallowed_table_deps_for_list(MEM_ROOT *mem_root,
+                                       TABLE_LIST *table_func,
+                                       List<TABLE_LIST> *join_list,
+                                       List<TABLE_LIST> *disallowed_tables)
 {
   TABLE_LIST *table;
   NESTED_JOIN *nested_join;
@@ -99,23 +104,25 @@ bool get_disallowed_table_deps_for_list(table_map table_func_bit,
   {
     if ((nested_join= table->nested_join))
     {
-      if (get_disallowed_table_deps_for_list(table_func_bit,
-                                             &nested_join->join_list,
-                                             disallowed_tables))
-        return true;
+      int res;
+      if ((res= get_disallowed_table_deps_for_list(mem_root, table_func,
+                                                   &nested_join->join_list,
+                                                   disallowed_tables)))
+        return res;
     }
     else
     {
-      *disallowed_tables |= table->table->map;
-      if (table_func_bit == table->table->map)
+      if (disallowed_tables->push_back(table, mem_root))
+        return -1;
+      if (table == table_func)
       {
         // This is the JSON_TABLE(...) that are we're computing dependencies
         // for.
-        return true;
+        return 1; // Finish the processing
       }
     }
   }
-  return false;
+  return 0; // Continue
 }
 
 
@@ -129,19 +136,31 @@ bool get_disallowed_table_deps_for_list(table_map table_func_bit,
     See get_disallowed_table_deps_for_list
 
   @return
-    Bitmap of tables that table function can NOT have references to.
+     NULL  - Out of memory
+     Other - A list of tables that the function cannot have references to. May
+             be empty.
 */
 
 static
-table_map get_disallowed_table_deps(JOIN *join, table_map table_func_bit)
+List<TABLE_LIST>* get_disallowed_table_deps(MEM_ROOT *mem_root,
+                                            SELECT_LEX *select,
+                                            TABLE_LIST *table_func)
 {
-  table_map disallowed_tables= 0;
-  if (!get_disallowed_table_deps_for_list(table_func_bit, join->join_list,
-                                          &disallowed_tables))
-  {
-    // We haven't found the table with table_func_bit in all tables?
-    DBUG_ASSERT(0);
-  }
+  List<TABLE_LIST> *disallowed_tables;
+
+  if (!(disallowed_tables = new List<TABLE_LIST>))
+    return NULL;
+
+  int res= get_disallowed_table_deps_for_list(mem_root, table_func,
+                                              select->join_list,
+                                              disallowed_tables);
+
+  // The collection process must have finished
+  DBUG_ASSERT(res != 0);
+
+  if (res == -1)
+    return NULL; // Out of memory
+
   return disallowed_tables;
 }
 
@@ -1034,48 +1053,56 @@ bool push_table_function_arg_context(LEX *lex, MEM_ROOT *alloc)
     Perform name-resolution phase tasks
 
   @detail
-    - The only argument that needs resolution is the JSON text
-    - Then, we need to set dependencies: if JSON_TABLE refers to table's
-      column, e.g.
+    The only argument that needs name resolution is the first parameter which
+    has the JSON text:
+
+      JSON_TABLE(json_doc, ... )
 
-         JSON_TABLE (t1.col ... ) AS t2
+    The argument may refer to other tables and uses special name resolution
+    rules (see get_disallowed_table_deps_for_list for details). This function
+    sets up Name_resolution_context object appropriately before calling
+    fix_fields for the argument.
 
-      then it can be computed only after table t1.
-    - The dependencies must not form a loop.
+  @return
+    false  OK
+    true   Fatal error
 */
 
-int Table_function_json_table::setup(THD *thd, TABLE_LIST *sql_table,
+bool Table_function_json_table::setup(THD *thd, TABLE_LIST *sql_table,
                                      SELECT_LEX *s_lex)
 {
-  TABLE *t= sql_table->table;
-
   thd->where= "JSON_TABLE argument";
-  {
-    bool save_is_item_list_lookup;
-    bool res;
-    save_is_item_list_lookup= s_lex->is_item_list_lookup;
-    s_lex->is_item_list_lookup= 0;
 
+  if (!m_context_setup_done)
+  {
+    m_context_setup_done= true;
     // Prepare the name resolution context. First, copy the context that is
     // used for name resolution of the WHERE clause
     *m_context= s_lex->context;
 
     // Then, restrict it to only allow to refer to tables that come before the
     // table function reference
-    m_context->ignored_tables= get_disallowed_table_deps(s_lex->join, t->map);
+    if (!(m_context->ignored_tables=
+            get_disallowed_table_deps(thd->stmt_arena->mem_root, s_lex,
+                                      sql_table)))
+      return TRUE; // Error
+  }
 
-    // Do the same what setup_without_group() does: do not count the referred
-    // fields in non_agg_field_used:
-    const bool saved_non_agg_field_used= s_lex->non_agg_field_used();
+  bool save_is_item_list_lookup;
+  save_is_item_list_lookup= s_lex->is_item_list_lookup;
+  s_lex->is_item_list_lookup= 0;
 
-    res= m_json->fix_fields_if_needed(thd, &m_json);
+  // Do the same what setup_without_group() does: do not count the referred
+  // fields in non_agg_field_used:
+  const bool saved_non_agg_field_used= s_lex->non_agg_field_used();
 
-    s_lex->is_item_list_lookup= save_is_item_list_lookup;
-    s_lex->set_non_agg_field_used(saved_non_agg_field_used);
+  bool res= m_json->fix_fields_if_needed(thd, &m_json);
 
-    if (res)
-      return TRUE;
-  }
+  s_lex->is_item_list_lookup= save_is_item_list_lookup;
+  s_lex->set_non_agg_field_used(saved_non_agg_field_used);
+
+  if (res)
+    return TRUE; // Error
 
   return FALSE;
 }

sql/json_table.h

@@ -198,7 +198,7 @@ class Table_function_json_table : public Sql_alloc
   List<Json_table_column> m_columns;
 
   /*** Name resolution functions ***/
-  int setup(THD *thd, TABLE_LIST *sql_table, SELECT_LEX *s_lex);
+  bool setup(THD *thd, TABLE_LIST *sql_table, SELECT_LEX *s_lex);
 
   int walk_items(Item_processor processor, bool walk_subquery,
                  void *argument);
@@ -226,7 +226,8 @@ class Table_function_json_table : public Sql_alloc
 
   /*** Construction interface to be used from the parser ***/
   Table_function_json_table(Item *json):
-    m_json(json)
+    m_json(json),
+    m_context_setup_done(false)
   {
     cur_parent= &m_nested_path;
     last_sibling_hook= &m_nested_path.m_nested;
@@ -250,6 +251,8 @@ class Table_function_json_table : public Sql_alloc
   /* Context to be used for resolving the first argument. */
   Name_resolution_context *m_context;
 
+  bool m_context_setup_done;
+
   /* Current NESTED PATH level being parsed */
   Json_table_nested_path *cur_parent;
 

sql/sql_base.cc

@@ -6101,7 +6101,8 @@ Field *
 find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
                         const char *name, size_t length,
                         const char *item_name, const char *db_name,
-                        const char *table_name, table_map ignored_tables,
+                        const char *table_name,
+                        ignored_tables_list_t ignored_tables,
                         Item **ref,
                         bool check_privileges, bool allow_rowid,
                         uint *cached_field_index_ptr,
@@ -6194,8 +6195,13 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
       TABLE_LIST *table;
       while ((table= it++))
       {
-        if (table->table && (table->table->map & ignored_tables))
+        /*
+          Check if the table is in the ignore list. Only base tables can be in
+          the ignore list.
+        */
+        if (table->table && ignored_list_includes_table(ignored_tables, table))
           continue;
+
         if ((fld= find_field_in_table_ref(thd, table, name, length, item_name,
                                           db_name, table_name, ignored_tables,
                                           ref, check_privileges, allow_rowid,
@@ -6322,8 +6328,8 @@ Field *find_field_in_table_sef(TABLE *table, const char *name)
     first_table           list of tables to be searched for item
     last_table            end of the list of tables to search for item. If NULL
                           then search to the end of the list 'first_table'.
-    ignored_tables        Bitmap of tables that should be ignored. Do not try
-                          to find the field in those.
+    ignored_tables        Set of tables that should be ignored. Do not try to
+                          find the field in those.
     ref			  if 'item' is resolved to a view field, ref is set to
                           point to the found view field
     report_error	  Degree of error reporting:
@@ -6351,7 +6357,7 @@ Field *find_field_in_table_sef(TABLE *table, const char *name)
 Field *
 find_field_in_tables(THD *thd, Item_ident *item,
                      TABLE_LIST *first_table, TABLE_LIST *last_table,
-                     table_map ignored_tables,
+                     ignored_tables_list_t ignored_tables,
 		     Item **ref, find_item_error_report_type report_error,
                      bool check_privileges, bool register_tree_change)
 {
@@ -6475,7 +6481,8 @@ find_field_in_tables(THD *thd, Item_ident *item,
   for (; cur_table != last_table ;
        cur_table= cur_table->next_name_resolution_table)
   {
-    if (cur_table->table && (cur_table->table->map & ignored_tables))
+    if (cur_table->table &&
+        ignored_list_includes_table(ignored_tables, cur_table))
       continue;
 
     Field *cur_field= find_field_in_table_ref(thd, cur_table, name, length,

sql/sql_base.h

@@ -195,14 +195,15 @@ bool fill_record(THD *thd, TABLE *table, Field **field, List<Item> &values,
 Field *
 find_field_in_tables(THD *thd, Item_ident *item,
                      TABLE_LIST *first_table, TABLE_LIST *last_table,
-                     table_map ignored_tables,
+                     ignored_tables_list_t ignored_tables,
                      Item **ref, find_item_error_report_type report_error,
                      bool check_privileges, bool register_tree_change);
 Field *
 find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
                         const char *name, size_t length,
                         const char *item_name, const char *db_name,
-                        const char *table_name, table_map ignored_tables,
+                        const char *table_name,
+                        ignored_tables_list_t ignored_tables,
                         Item **ref, bool check_privileges, bool allow_rowid,
                         uint *cached_field_index_ptr,
                         bool register_tree_change, TABLE_LIST **actual_table);