summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSergei Petrunia <psergey@askmonty.org>2021-04-16 19:50:08 +0300
committerAlexey Botchkov <holyfoot@askmonty.org>2021-04-21 10:21:48 +0400
commitb0817ff8def0104bf2f82c98b10480a5854a69fc (patch)
tree6d4035d1eb76d1850eaf51bc3d2970818b17c674
parent0984b8ed0876f9c7046964b000a8bb27f2587b14 (diff)
downloadmariadb-git-b0817ff8def0104bf2f82c98b10480a5854a69fc.tar.gz
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.
-rw-r--r--mysql-test/suite/json/r/json_table.result17
-rw-r--r--mysql-test/suite/json/t/json_table.test13
-rw-r--r--sql/item.cc15
-rw-r--r--sql/item.h7
-rw-r--r--sql/json_table.cc125
-rw-r--r--sql/json_table.h7
-rw-r--r--sql/sql_base.cc19
-rw-r--r--sql/sql_base.h5
8 files changed, 147 insertions, 61 deletions
diff --git a/mysql-test/suite/json/r/json_table.result b/mysql-test/suite/json/r/json_table.result
index 6cb197890c1..a996401496f 100644
--- a/mysql-test/suite/json/r/json_table.result
+++ b/mysql-test/suite/json/r/json_table.result
@@ -790,6 +790,23 @@ ON t1.a = tt.b;
a b c o
1 1 {} 1
2 2 [] NULL
+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;
+a b c o
+1 1 {} 1
+2 2 [] NULL
+execute s;
+a b c o
+1 1 {} 1
+2 2 [] NULL
DROP VIEW v2;
DROP TABLE t1, t2;
#
diff --git a/mysql-test/suite/json/t/json_table.test b/mysql-test/suite/json/t/json_table.test
index 41233c8a8ea..39c97f73496 100644
--- a/mysql-test/suite/json/t/json_table.test
+++ b/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;
diff --git a/sql/item.cc b/sql/item.cc
index e02476da4ed..152f36bb9e8 100644
--- a/sql/item.cc
+++ b/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;
+}
diff --git a/sql/item.h b/sql/item.h
index cfa10581f9b..dbf950b4894 100644
--- a/sql/item.h
+++ b/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)
{}
diff --git a/sql/json_table.cc b/sql/json_table.cc
index a16adc25f08..c79abf942fa 100644
--- a/sql/json_table.cc
+++ b/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;
}
diff --git a/sql/json_table.h b/sql/json_table.h
index 09e4295d80c..beae5405d25 100644
--- a/sql/json_table.h
+++ b/sql/json_table.h
@@ -198,7 +198,7 @@ public:
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 @@ public:
/*** 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 @@ private:
/* 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;
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 42ab81048ec..6f6f9da7486 100644
--- a/sql/sql_base.cc
+++ b/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,
diff --git a/sql/sql_base.h b/sql/sql_base.h
index 1836c07497a..922c61ca123 100644
--- a/sql/sql_base.h
+++ b/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);