- commits - lists.mariadb.org

[Commits] 6bb06db4fcb: Apply patch: Consolidate rnd_init/rnd_next/rnd_end into index_*
by psergey 17 May '21

17 May '21

revision-id: 6bb06db4fcb091dcafb0f896a962b7321078b44e (percona-202102-45-g6bb06db4fcb) parent(s): dea55321f0f03df1a29130aa6a2d38a0f9b6e39e author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-17 16:48:30 +0300 message: Apply patch: Consolidate rnd_init/rnd_next/rnd_end into index_* Summary: Instead of maintaining separate code paths for full table scans, merge them into full index scan code paths. Full table scans are just full index scans on the primary key anyway. Test Plan: mtr Reviewers: luqun, herman, yzha Subscribers: pgl, vinaybhat Differential Revision: https://phabricator.intern.facebook.com/D23358422 --- storage/rocksdb/ha_rocksdb.cc | 157 +++++++++++++++++++----------------------- storage/rocksdb/ha_rocksdb.h | 11 ++- 2 files changed, 78 insertions(+), 90 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 7f7c6a79a2e..077b5116a5e 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -7009,7 +7009,7 @@ void ha_rocksdb::update_auto_incr_val_from_field() { int ha_rocksdb::load_hidden_pk_value() { const int save_active_index = active_index; - active_index = m_tbl_def->m_key_count - 1; + active_index = MAX_KEY; const uint8 save_table_status = table->m_status; Rdb_transaction *const tx = get_or_create_tx(table->in_use); @@ -9218,7 +9218,7 @@ int ha_rocksdb::index_read_map(uchar *const buf, const uchar *const key, } ha_statistic_increment(&System_status_var::ha_read_key_count); - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; const uint actual_key_parts = kd.get_key_parts(); bool using_full_key = is_using_full_key(keypart_map, actual_key_parts); @@ -9415,7 +9415,7 @@ int ha_rocksdb::index_read_map(uchar *const buf, const uchar *const key, int ha_rocksdb::find_icp_matching_index_rec(const bool move_forward, uchar *const buf) { if (pushed_idx_cond && pushed_idx_cond_keyno == active_index) { - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; THD *thd = ha_thd(); while (1) { @@ -9835,19 +9835,10 @@ int ha_rocksdb::records_from_index(ha_rows *num_rows, uint index) { */ int ha_rocksdb::index_next(uchar *const buf) { DBUG_ENTER_FUNC(); - check_build_decoder(); - bool moves_forward = true; ha_statistic_increment(&System_status_var::ha_read_next_count); - if (m_key_descr_arr[active_index]->m_is_reverse_cf) { - moves_forward = false; - } - - int rc = index_next_with_direction(buf, moves_forward); - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; - - DBUG_RETURN(rc); + DBUG_RETURN(index_next_intern(buf)); } /** @@ -9857,19 +9848,10 @@ int ha_rocksdb::index_next(uchar *const buf) { */ int ha_rocksdb::index_prev(uchar *const buf) { DBUG_ENTER_FUNC(); - check_build_decoder(); - bool moves_forward = false; ha_statistic_increment(&System_status_var::ha_read_prev_count); - if (m_key_descr_arr[active_index]->m_is_reverse_cf) { - moves_forward = true; - } - - int rc = index_next_with_direction(buf, moves_forward); - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; - - DBUG_RETURN(rc); + DBUG_RETURN(index_prev_intern(buf)); } int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { @@ -9877,7 +9859,7 @@ int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { int rc; - if (active_index == pk_index(table, m_tbl_def)) { + if (active_index == table->s->primary_key) { rc = rnd_next_with_direction(buf, move_forward); } else { THD *thd = ha_thd(); @@ -9896,7 +9878,7 @@ int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { m_scan_it->Prev(); } } - rc = rocksdb_skip_expired_records(*m_key_descr_arr[active_index], + rc = rocksdb_skip_expired_records(*m_key_descr_arr[active_index_pos()], m_scan_it, !move_forward); if (rc != HA_EXIT_SUCCESS) { break; @@ -10017,7 +9999,7 @@ int ha_rocksdb::index_read_intern(uchar *const buf, bool first) { uint key_size; int rc; - if (is_pk(active_index, table, m_tbl_def)) { + if (active_index == table->s->primary_key) { key = m_pk_packed_tuple; } else { key = m_sk_packed_tuple; @@ -10025,7 +10007,7 @@ int ha_rocksdb::index_read_intern(uchar *const buf, bool first) { DBUG_ASSERT(key != nullptr); - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; bool backwards = (first && kd.m_is_reverse_cf) || (!first && !kd.m_is_reverse_cf); @@ -10166,6 +10148,11 @@ uint ha_rocksdb::pk_index(const TABLE *const table_arg, : table_arg->s->primary_key; } +/* Returns the index into m_key_descr_arr array based on active_index */ +uint ha_rocksdb::active_index_pos() { + return active_index == MAX_KEY ? m_tbl_def->m_key_count - 1 : active_index; +} + /* Returns true if given index number is a primary key */ bool ha_rocksdb::is_pk(const uint index, const TABLE *const table_arg, const Rdb_tbl_def *const tbl_def_arg) { @@ -11292,50 +11279,21 @@ void ha_rocksdb::release_scan_iterator() { } } -void ha_rocksdb::setup_iterator_for_rnd_scan() { - uint key_size; - - int key_start_matching_bytes = - m_pk_descr->get_first_key(m_pk_packed_tuple, &key_size); - - rocksdb::Slice table_key((const char *)m_pk_packed_tuple, key_size); - - setup_scan_iterator(*m_pk_descr, &table_key, false, key_start_matching_bytes); - m_scan_it->Seek(table_key); - m_skip_scan_it_next_call = true; -} - /** @return HA_EXIT_SUCCESS OK other HA_ERR error code (can be SE-specific) */ -int ha_rocksdb::rnd_init(bool scan) { +int ha_rocksdb::rnd_init(bool) { DBUG_ENTER_FUNC(); m_need_build_decoder = true; active_index = table->s->primary_key; - THD *thd = ha_thd(); - if (thd && thd->killed) { - DBUG_RETURN(HA_ERR_QUERY_INTERRUPTED); - } - - Rdb_transaction *const tx = get_or_create_tx(table->in_use); - - if (scan) { - m_rnd_scan_is_new_snapshot = !tx->has_snapshot(); - setup_iterator_for_rnd_scan(); - } else { - /* We don't need any preparations for rnd_pos() calls. */ - } - - // If m_lock_rows is on then we will be doing a get_for_update when accessing - // the index, so don't acquire the snapshot right away. Otherwise acquire - // the snapshot immediately. - tx->acquire_snapshot(m_lock_rows == RDB_LOCK_NONE); - - DBUG_RETURN(HA_EXIT_SUCCESS); + m_rnd_scan_started = false; + DBUG_RETURN( + index_init(has_hidden_pk(table) ? MAX_KEY : pk_index(table, m_tbl_def), + false /* sorted */)); } /** @@ -11350,20 +11308,22 @@ int ha_rocksdb::rnd_next(uchar *const buf) { int rc; ha_statistic_increment(&System_status_var::ha_read_rnd_next_count); - for (;;) { - rc = rnd_next_with_direction(buf, true); - if (!should_recreate_snapshot(rc, m_rnd_scan_is_new_snapshot)) { - break; /* exit the loop */ + + /* + Since order does not matter, the scan will occur go with natural index + order. + */ + bool is_reverse_cf = m_key_descr_arr[active_index_pos()]->m_is_reverse_cf; + if (!m_rnd_scan_started) { + rc = index_read_intern(buf, !is_reverse_cf /* first */); + m_rnd_scan_started = true; + } else { + if (is_reverse_cf) { + rc = index_prev_intern(buf); + } else { + rc = index_next_intern(buf); } - // release the snapshot and iterator and then regenerate them - Rdb_transaction *tx = get_or_create_tx(table->in_use); - tx->release_snapshot(); - release_scan_iterator(); - setup_iterator_for_rnd_scan(); } - - m_rnd_scan_is_new_snapshot = false; - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; DBUG_RETURN(rc); @@ -11492,12 +11452,9 @@ int ha_rocksdb::rnd_next_with_direction(uchar *const buf, bool move_forward) { int ha_rocksdb::rnd_end() { DBUG_ENTER_FUNC(); - + DBUG_RETURN(index_end()); m_need_build_decoder = false; - release_scan_iterator(); - - DBUG_RETURN(HA_EXIT_SUCCESS); } void ha_rocksdb::build_decoder() { @@ -11561,6 +11518,34 @@ int ha_rocksdb::index_end() { DBUG_RETURN(HA_EXIT_SUCCESS); } +int ha_rocksdb::index_next_intern(uchar *const buf) { + DBUG_ENTER_FUNC(); + + bool moves_forward = true; + if (m_key_descr_arr[active_index_pos()]->m_is_reverse_cf) { + moves_forward = false; + } + + int rc = index_next_with_direction(buf, moves_forward); + if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; + + DBUG_RETURN(rc); +} + +int ha_rocksdb::index_prev_intern(uchar *const buf) { + DBUG_ENTER_FUNC(); + + bool moves_forward = false; + if (m_key_descr_arr[active_index_pos()]->m_is_reverse_cf) { + moves_forward = true; + } + + int rc = index_next_with_direction(buf, moves_forward); + if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; + + DBUG_RETURN(rc); +} + /** Called by the partition manager for truncating tables. @@ -14025,23 +14010,21 @@ int ha_rocksdb::inplace_populate_sk( } /* - Note: We pass in the currently existing table + tbl_def object here, + Note: We use the currently existing table + tbl_def object here, as the pk index position may have changed in the case of hidden primary keys. */ - const uint pk = pk_index(table, m_tbl_def); - res = ha_index_init(pk, true); - if (res) DBUG_RETURN(res); + ha_rnd_init(true /* scan */); /* Scan each record in the primary key in order */ - for (res = index_first(table->record[0]); res == 0; - res = index_next(table->record[0])) { + for (res = ha_rnd_next(table->record[0]); res == 0; + res = ha_rnd_next(table->record[0])) { longlong hidden_pk_id = 0; if (hidden_pk_exists && (res = read_hidden_pk_id_from_rowkey(&hidden_pk_id))) { // NO_LINT_DEBUG sql_print_error("Error retrieving hidden pk id."); - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } @@ -14062,7 +14045,7 @@ int ha_rocksdb::inplace_populate_sk( disk in sorted chunks. */ if ((res = rdb_merge.add(key, val))) { - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } } @@ -14070,11 +14053,11 @@ int ha_rocksdb::inplace_populate_sk( if (res != HA_ERR_END_OF_FILE) { // NO_LINT_DEBUG sql_print_error("Error retrieving index entry from primary key."); - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } - ha_index_end(); + ha_rnd_end(); /* Perform an n-way merge of n sorted buffers on disk, then writes all diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index 18c40fc1ca9..ba62521fa79 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/storage/rocksdb/ha_rocksdb.h @@ -282,8 +282,7 @@ class ha_rocksdb : public my_core::handler { bool m_skip_scan_it_next_call; - /* true means we are accessing the first row after a snapshot was created */ - bool m_rnd_scan_is_new_snapshot; + bool m_rnd_scan_started; /* true means INSERT ON DUPLICATE KEY UPDATE. In such case we can optimize by @@ -331,7 +330,6 @@ class ha_rocksdb : public my_core::handler { MY_ATTRIBUTE((__nonnull__(2, 3), __warn_unused_result__)); int secondary_index_read(const int keyno, uchar *const buf) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); - void setup_iterator_for_rnd_scan(); static void setup_iterator_bounds(const Rdb_key_def &kd, const rocksdb::Slice &eq_cond, size_t bound_len, uchar *const lower_bound, @@ -539,6 +537,8 @@ class ha_rocksdb : public my_core::handler { const Rdb_tbl_def *const tbl_def_arg) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); + uint active_index_pos() MY_ATTRIBUTE((__warn_unused_result__)); + static bool is_pk(const uint index, const TABLE *table_arg, const Rdb_tbl_def *tbl_def_arg) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); @@ -644,6 +644,11 @@ class ha_rocksdb : public my_core::handler { int index_last(uchar *const buf) override MY_ATTRIBUTE((__warn_unused_result__)); + int index_next_intern(uchar *const buf) + MY_ATTRIBUTE((__warn_unused_result__)); + int index_prev_intern(uchar *const buf) + MY_ATTRIBUTE((__warn_unused_result__)); + class Item *idx_cond_push(uint keyno, class Item *const idx_cond) override; /* Default implementation from cancel_pushed_idx_cond() suits us

1 0

[Commits] 31a395578da: Apply patch: Consolidate rnd_init/rnd_next/rnd_end into index_*
by psergey 17 May '21

17 May '21

revision-id: 31a395578da893e201ca58a4e20cd43596c9c295 (percona-202102-45-g31a395578da) parent(s): dea55321f0f03df1a29130aa6a2d38a0f9b6e39e author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-17 16:40:35 +0300 message: Apply patch: Consolidate rnd_init/rnd_next/rnd_end into index_* Summary: Instead of maintaining separate code paths for full table scans, merge them into full index scan code paths. Full table scans are just full index scans on the primary key anyway. Test Plan: mtr Reviewers: luqun, herman, yzha Subscribers: pgl, vinaybhat Differential Revision: https://phabricator.intern.facebook.com/D23358422 --- storage/rocksdb/ha_rocksdb.cc | 153 +++++++++++++++++++----------------------- storage/rocksdb/ha_rocksdb.h | 11 ++- 2 files changed, 77 insertions(+), 87 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 7f7c6a79a2e..a5afc625139 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -7009,7 +7009,7 @@ void ha_rocksdb::update_auto_incr_val_from_field() { int ha_rocksdb::load_hidden_pk_value() { const int save_active_index = active_index; - active_index = m_tbl_def->m_key_count - 1; + active_index = MAX_KEY; const uint8 save_table_status = table->m_status; Rdb_transaction *const tx = get_or_create_tx(table->in_use); @@ -9218,7 +9218,7 @@ int ha_rocksdb::index_read_map(uchar *const buf, const uchar *const key, } ha_statistic_increment(&System_status_var::ha_read_key_count); - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; const uint actual_key_parts = kd.get_key_parts(); bool using_full_key = is_using_full_key(keypart_map, actual_key_parts); @@ -9415,7 +9415,7 @@ int ha_rocksdb::index_read_map(uchar *const buf, const uchar *const key, int ha_rocksdb::find_icp_matching_index_rec(const bool move_forward, uchar *const buf) { if (pushed_idx_cond && pushed_idx_cond_keyno == active_index) { - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; THD *thd = ha_thd(); while (1) { @@ -9835,19 +9835,10 @@ int ha_rocksdb::records_from_index(ha_rows *num_rows, uint index) { */ int ha_rocksdb::index_next(uchar *const buf) { DBUG_ENTER_FUNC(); - check_build_decoder(); - bool moves_forward = true; ha_statistic_increment(&System_status_var::ha_read_next_count); - if (m_key_descr_arr[active_index]->m_is_reverse_cf) { - moves_forward = false; - } - - int rc = index_next_with_direction(buf, moves_forward); - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; - - DBUG_RETURN(rc); + DBUG_RETURN(index_next_intern(buf)); } /** @@ -9857,19 +9848,10 @@ int ha_rocksdb::index_next(uchar *const buf) { */ int ha_rocksdb::index_prev(uchar *const buf) { DBUG_ENTER_FUNC(); - check_build_decoder(); - bool moves_forward = false; ha_statistic_increment(&System_status_var::ha_read_prev_count); - if (m_key_descr_arr[active_index]->m_is_reverse_cf) { - moves_forward = true; - } - - int rc = index_next_with_direction(buf, moves_forward); - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; - - DBUG_RETURN(rc); + DBUG_RETURN(index_prev_intern(buf)); } int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { @@ -9877,7 +9859,7 @@ int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { int rc; - if (active_index == pk_index(table, m_tbl_def)) { + if (active_index == table->s->primary_key) { rc = rnd_next_with_direction(buf, move_forward); } else { THD *thd = ha_thd(); @@ -9896,7 +9878,7 @@ int ha_rocksdb::index_next_with_direction(uchar *const buf, bool move_forward) { m_scan_it->Prev(); } } - rc = rocksdb_skip_expired_records(*m_key_descr_arr[active_index], + rc = rocksdb_skip_expired_records(*m_key_descr_arr[active_index_pos()], m_scan_it, !move_forward); if (rc != HA_EXIT_SUCCESS) { break; @@ -10017,7 +9999,7 @@ int ha_rocksdb::index_read_intern(uchar *const buf, bool first) { uint key_size; int rc; - if (is_pk(active_index, table, m_tbl_def)) { + if (active_index == table->s->primary_key) { key = m_pk_packed_tuple; } else { key = m_sk_packed_tuple; @@ -10025,7 +10007,7 @@ int ha_rocksdb::index_read_intern(uchar *const buf, bool first) { DBUG_ASSERT(key != nullptr); - const Rdb_key_def &kd = *m_key_descr_arr[active_index]; + const Rdb_key_def &kd = *m_key_descr_arr[active_index_pos()]; bool backwards = (first && kd.m_is_reverse_cf) || (!first && !kd.m_is_reverse_cf); @@ -10166,6 +10148,11 @@ uint ha_rocksdb::pk_index(const TABLE *const table_arg, : table_arg->s->primary_key; } +/* Returns the index into m_key_descr_arr array based on active_index */ +uint ha_rocksdb::active_index_pos() { + return active_index == MAX_KEY ? m_tbl_def->m_key_count - 1 : active_index; +} + /* Returns true if given index number is a primary key */ bool ha_rocksdb::is_pk(const uint index, const TABLE *const table_arg, const Rdb_tbl_def *const tbl_def_arg) { @@ -11292,50 +11279,21 @@ void ha_rocksdb::release_scan_iterator() { } } -void ha_rocksdb::setup_iterator_for_rnd_scan() { - uint key_size; - - int key_start_matching_bytes = - m_pk_descr->get_first_key(m_pk_packed_tuple, &key_size); - - rocksdb::Slice table_key((const char *)m_pk_packed_tuple, key_size); - - setup_scan_iterator(*m_pk_descr, &table_key, false, key_start_matching_bytes); - m_scan_it->Seek(table_key); - m_skip_scan_it_next_call = true; -} - /** @return HA_EXIT_SUCCESS OK other HA_ERR error code (can be SE-specific) */ -int ha_rocksdb::rnd_init(bool scan) { +int ha_rocksdb::rnd_init(bool) { DBUG_ENTER_FUNC(); m_need_build_decoder = true; active_index = table->s->primary_key; - THD *thd = ha_thd(); - if (thd && thd->killed) { - DBUG_RETURN(HA_ERR_QUERY_INTERRUPTED); - } - - Rdb_transaction *const tx = get_or_create_tx(table->in_use); - - if (scan) { - m_rnd_scan_is_new_snapshot = !tx->has_snapshot(); - setup_iterator_for_rnd_scan(); - } else { - /* We don't need any preparations for rnd_pos() calls. */ - } - - // If m_lock_rows is on then we will be doing a get_for_update when accessing - // the index, so don't acquire the snapshot right away. Otherwise acquire - // the snapshot immediately. - tx->acquire_snapshot(m_lock_rows == RDB_LOCK_NONE); - - DBUG_RETURN(HA_EXIT_SUCCESS); + m_rnd_scan_started = false; + DBUG_RETURN( + index_init(has_hidden_pk(table) ? MAX_KEY : pk_index(table, m_tbl_def), + false /* sorted */)); } /** @@ -11350,20 +11308,22 @@ int ha_rocksdb::rnd_next(uchar *const buf) { int rc; ha_statistic_increment(&System_status_var::ha_read_rnd_next_count); - for (;;) { - rc = rnd_next_with_direction(buf, true); - if (!should_recreate_snapshot(rc, m_rnd_scan_is_new_snapshot)) { - break; /* exit the loop */ + + /* + Since order does not matter, the scan will occur go with natural index + order. + */ + bool is_reverse_cf = m_key_descr_arr[active_index_pos()]->m_is_reverse_cf; + if (!m_rnd_scan_started) { + rc = index_read_intern(buf, !is_reverse_cf /* first */); + m_rnd_scan_started = true; + } else { + if (is_reverse_cf) { + rc = index_prev_intern(buf); + } else { + rc = index_next_intern(buf); } - // release the snapshot and iterator and then regenerate them - Rdb_transaction *tx = get_or_create_tx(table->in_use); - tx->release_snapshot(); - release_scan_iterator(); - setup_iterator_for_rnd_scan(); } - - m_rnd_scan_is_new_snapshot = false; - if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; DBUG_RETURN(rc); @@ -11492,12 +11452,9 @@ int ha_rocksdb::rnd_next_with_direction(uchar *const buf, bool move_forward) { int ha_rocksdb::rnd_end() { DBUG_ENTER_FUNC(); - + DBUG_RETURN(index_end()); m_need_build_decoder = false; - release_scan_iterator(); - - DBUG_RETURN(HA_EXIT_SUCCESS); } void ha_rocksdb::build_decoder() { @@ -11561,6 +11518,34 @@ int ha_rocksdb::index_end() { DBUG_RETURN(HA_EXIT_SUCCESS); } +int ha_rocksdb::index_next_intern(uchar *const buf) { + DBUG_ENTER_FUNC(); + + bool moves_forward = true; + if (m_key_descr_arr[active_index_pos()]->m_is_reverse_cf) { + moves_forward = false; + } + + int rc = index_next_with_direction(buf, moves_forward); + if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; + + DBUG_RETURN(rc); +} + +int ha_rocksdb::index_prev_intern(uchar *const buf) { + DBUG_ENTER_FUNC(); + + bool moves_forward = false; + if (m_key_descr_arr[active_index_pos()]->m_is_reverse_cf) { + moves_forward = true; + } + + int rc = index_next_with_direction(buf, moves_forward); + if (rc == HA_ERR_KEY_NOT_FOUND) rc = HA_ERR_END_OF_FILE; + + DBUG_RETURN(rc); +} + /** Called by the partition manager for truncating tables. @@ -14025,7 +14010,7 @@ int ha_rocksdb::inplace_populate_sk( } /* - Note: We pass in the currently existing table + tbl_def object here, + Note: We use the currently existing table + tbl_def object here, as the pk index position may have changed in the case of hidden primary keys. */ @@ -14034,14 +14019,14 @@ int ha_rocksdb::inplace_populate_sk( if (res) DBUG_RETURN(res); /* Scan each record in the primary key in order */ - for (res = index_first(table->record[0]); res == 0; - res = index_next(table->record[0])) { + for (res = ha_rnd_next(table->record[0]); res == 0; + res = ha_rnd_next(table->record[0])) { longlong hidden_pk_id = 0; if (hidden_pk_exists && (res = read_hidden_pk_id_from_rowkey(&hidden_pk_id))) { // NO_LINT_DEBUG sql_print_error("Error retrieving hidden pk id."); - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } @@ -14062,7 +14047,7 @@ int ha_rocksdb::inplace_populate_sk( disk in sorted chunks. */ if ((res = rdb_merge.add(key, val))) { - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } } @@ -14070,11 +14055,11 @@ int ha_rocksdb::inplace_populate_sk( if (res != HA_ERR_END_OF_FILE) { // NO_LINT_DEBUG sql_print_error("Error retrieving index entry from primary key."); - ha_index_end(); + ha_rnd_end(); DBUG_RETURN(res); } - ha_index_end(); + ha_rnd_end(); /* Perform an n-way merge of n sorted buffers on disk, then writes all diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index 18c40fc1ca9..ba62521fa79 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/storage/rocksdb/ha_rocksdb.h @@ -282,8 +282,7 @@ class ha_rocksdb : public my_core::handler { bool m_skip_scan_it_next_call; - /* true means we are accessing the first row after a snapshot was created */ - bool m_rnd_scan_is_new_snapshot; + bool m_rnd_scan_started; /* true means INSERT ON DUPLICATE KEY UPDATE. In such case we can optimize by @@ -331,7 +330,6 @@ class ha_rocksdb : public my_core::handler { MY_ATTRIBUTE((__nonnull__(2, 3), __warn_unused_result__)); int secondary_index_read(const int keyno, uchar *const buf) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); - void setup_iterator_for_rnd_scan(); static void setup_iterator_bounds(const Rdb_key_def &kd, const rocksdb::Slice &eq_cond, size_t bound_len, uchar *const lower_bound, @@ -539,6 +537,8 @@ class ha_rocksdb : public my_core::handler { const Rdb_tbl_def *const tbl_def_arg) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); + uint active_index_pos() MY_ATTRIBUTE((__warn_unused_result__)); + static bool is_pk(const uint index, const TABLE *table_arg, const Rdb_tbl_def *tbl_def_arg) MY_ATTRIBUTE((__nonnull__, __warn_unused_result__)); @@ -644,6 +644,11 @@ class ha_rocksdb : public my_core::handler { int index_last(uchar *const buf) override MY_ATTRIBUTE((__warn_unused_result__)); + int index_next_intern(uchar *const buf) + MY_ATTRIBUTE((__warn_unused_result__)); + int index_prev_intern(uchar *const buf) + MY_ATTRIBUTE((__warn_unused_result__)); + class Item *idx_cond_push(uint keyno, class Item *const idx_cond) override; /* Default implementation from cancel_pushed_idx_cond() suits us

1 0

[Commits] 9317dc96f03: MDEV-25631: Crash in st_select_lex::mark_as_dependent with VIEW, aggregate and subquery
by psergey 16 May '21

16 May '21

revision-id: 9317dc96f03f558dc20d41f7dcf003e6ebf6a2b4 (mariadb-10.6.0-52-g9317dc96f03) parent(s): bee1bb056dd5350c967dda65efb75e3a171e649a author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-16 12:30:40 +0300 message: MDEV-25631: Crash in st_select_lex::mark_as_dependent with VIEW, aggregate and subquery Name resolution code has checks like this one: if (thd->lex->in_sum_func && thd->lex->in_sum_func->nest_level >= select->nest_level) ... This fails to take into account the fact SELECT_LEX::nest_level is local to each VIEW. Adjust the check so that it only succeeds when the select and the aggregate function being considered are from the same VIEW (or both from the top-level query): add this: + thd->lex->in_sum_func->nest_level_base == select->nest_level_base && Note: this patch only modifies one such check. There are many, should they all be adjusted in the same way? --- mysql-test/main/subselect4.result | 11 +++++++++++ mysql-test/main/subselect4.test | 15 +++++++++++++++ sql/item.cc | 3 +++ sql/item_sum.cc | 1 + sql/item_sum.h | 1 + 5 files changed, 31 insertions(+) diff --git a/mysql-test/main/subselect4.result b/mysql-test/main/subselect4.result index 3bfb755120b..182235c039b 100644 --- a/mysql-test/main/subselect4.result +++ b/mysql-test/main/subselect4.result @@ -2898,3 +2898,14 @@ id select_type table type possible_keys key key_len ref rows Extra 2 MATERIALIZED t2 ALL NULL NULL NULL NULL 100 drop table t0, t1, t2; # End of 10.4 tests +# +# MDEV-25631: Crash in st_select_lex::mark_as_dependent with VIEW, aggregate and subquery +# +CREATE TABLE t1 (i1 int); +insert into t1 values (1),(2),(3); +CREATE VIEW v1 AS +SELECT t1.i1 FROM (t1 a JOIN t1 ON (t1.i1 = (SELECT t1.i1 FROM t1 b))); +SELECT 1 FROM (SELECT count(((SELECT i1 FROM v1))) FROM v1) dt ; +ERROR 21000: Subquery returns more than 1 row +DROP VIEW v1; +DROP TABLE t1; diff --git a/mysql-test/main/subselect4.test b/mysql-test/main/subselect4.test index a1a4108de37..bd18ec5f5c9 100644 --- a/mysql-test/main/subselect4.test +++ b/mysql-test/main/subselect4.test @@ -2398,3 +2398,18 @@ select * from t1 where t1.a in (select t2.a from t2 order by t2.b); drop table t0, t1, t2; --echo # End of 10.4 tests + +--echo # +--echo # MDEV-25631: Crash in st_select_lex::mark_as_dependent with VIEW, aggregate and subquery +--echo # + +CREATE TABLE t1 (i1 int); +insert into t1 values (1),(2),(3); #not important +CREATE VIEW v1 AS +SELECT t1.i1 FROM (t1 a JOIN t1 ON (t1.i1 = (SELECT t1.i1 FROM t1 b))); + +--error ER_SUBQUERY_NO_1_ROW +SELECT 1 FROM (SELECT count(((SELECT i1 FROM v1))) FROM v1) dt ; + +DROP VIEW v1; +DROP TABLE t1; diff --git a/sql/item.cc b/sql/item.cc index 5cdbf52e829..a4c16c53e5e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -5608,9 +5608,12 @@ Item_field::fix_outer_field(THD *thd, Field **from_field, Item **reference) max_arg_level for the function if it's needed. */ if (thd->lex->in_sum_func && + thd->lex->in_sum_func->nest_level_base == select->nest_level_base && thd->lex->in_sum_func->nest_level >= select->nest_level) { Item::Type ref_type= (*reference)->type(); + // psergey-todo: check if in_sum_func "has" the same + // nest_level_base as we do.. set_if_bigger(thd->lex->in_sum_func->max_arg_level, select->nest_level); set_field(*from_field); diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 537eaaf8dcd..23b6f739333 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -92,6 +92,7 @@ bool Item_sum::init_sum_func_check(THD *thd) /* Save a pointer to object to be used in items for nested set functions */ thd->lex->in_sum_func= this; nest_level= thd->lex->current_select->nest_level; + nest_level_base= thd->lex->current_select->nest_level_base; ref_by= 0; aggr_level= -1; aggr_sel= NULL; diff --git a/sql/item_sum.h b/sql/item_sum.h index 118f78ec5c1..10aa658c5e2 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -364,6 +364,7 @@ class Item_sum :public Item_func_or_sum Item_sum *in_sum_func; /* embedding set function if any */ st_select_lex * aggr_sel; /* select where the function is aggregated */ int8 nest_level; /* number of the nesting level of the set function */ + st_select_lex_unit *nest_level_base; int8 aggr_level; /* nesting level of the aggregating subquery */ int8 max_arg_level; /* max level of unbound column references */ int8 max_sum_func_level;/* max level of aggregation for embedded functions */

1 0

[Commits] 677f1ef: MDEV-25682 Explain shows an execution plan different from actually executed
by IgorBabaev 14 May '21

14 May '21

revision-id: 677f1ef6f00793b3ad2a42b4e6f0fcbb7cd0e39d (mariadb-10.2.31-950-g677f1ef) parent(s): e607f3398c69147299884d3814cf063d2e7516ce author: Igor Babaev committer: Igor Babaev timestamp: 2021-05-14 16:43:36 -0700 message: MDEV-25682 Explain shows an execution plan different from actually executed If a select query contained an ORDER BY clause that followed a LIMIT clause or an ORDER BY clause or ORDER BY with LIMIT the EXPLAIN output for the query showed an execution plan different from that was actually executed. Approved by Roman Nozdrin <roman.nozdrin(a)mariadb.com> --- mysql-test/r/order_by.result | 25 +++++++++++++++++++++++++ mysql-test/t/order_by.test | 16 ++++++++++++++++ sql/sql_select.cc | 2 +- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/order_by.result b/mysql-test/r/order_by.result index b144101..39b4e25 100644 --- a/mysql-test/r/order_by.result +++ b/mysql-test/r/order_by.result @@ -3460,4 +3460,29 @@ SET max_length_for_sort_data=@save_max_length_for_sort_data; SET max_sort_length= @save_max_sort_length; SET sql_select_limit= @save_sql_select_limit; DROP TABLE t1; +# +# MDEV-25682: EXPLAIN for SELECT with ORDER BY after [ORDER BY] LIMIT +# +create table t1 (a int); +insert into t1 values (3), (7), (1); +explain (select a from t1 limit 2) order by a desc; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 3 +NULL UNION RESULT <union1> ALL NULL NULL NULL NULL NULL Using filesort +(select a from t1 limit 2) order by a desc; +a +7 +3 +create table t2 (a int, b int); +insert into t2 values (3,70), (7,10), (1,40), (4,30); +explain (select b,a from t2 order by a limit 3) order by b desc; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 4 Using filesort +NULL UNION RESULT <union1> ALL NULL NULL NULL NULL NULL Using filesort +(select b,a from t2 order by a limit 3) order by b desc; +b a +70 3 +40 1 +30 4 +drop table t1,t2; # End of 10.2 tests diff --git a/mysql-test/t/order_by.test b/mysql-test/t/order_by.test index 36c25ed..4e50fc5 100644 --- a/mysql-test/t/order_by.test +++ b/mysql-test/t/order_by.test @@ -2293,4 +2293,20 @@ SET max_sort_length= @save_max_sort_length; SET sql_select_limit= @save_sql_select_limit; DROP TABLE t1; +--echo # +--echo # MDEV-25682: EXPLAIN for SELECT with ORDER BY after [ORDER BY] LIMIT +--echo # + +create table t1 (a int); +insert into t1 values (3), (7), (1); +explain (select a from t1 limit 2) order by a desc; +(select a from t1 limit 2) order by a desc; + +create table t2 (a int, b int); +insert into t2 values (3,70), (7,10), (1,40), (4,30); +explain (select b,a from t2 order by a limit 3) order by b desc; +(select b,a from t2 order by a limit 3) order by b desc; + +drop table t1,t2; + --echo # End of 10.2 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index b85bd31..ce70620 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -25332,7 +25332,7 @@ bool mysql_explain_union(THD *thd, SELECT_LEX_UNIT *unit, select_result *result) sl->options|= SELECT_DESCRIBE; } - if (unit->is_union()) + if (unit->is_union() || unit->fake_select_lex) { if (unit->union_needs_tmp_table() && unit->fake_select_lex) {

1 0

[Commits] 77dfdf839c6: MDEV-25629: Crash in get_sort_by_table() in subquery with order by having outer ref
by psergey 14 May '21

14 May '21

revision-id: 77dfdf839c61d84e913710da7b3647cd02e4ab58 (mariadb-10.2.31-950-g77dfdf839c6) parent(s): e607f3398c69147299884d3814cf063d2e7516ce author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-14 20:43:21 +0300 message: MDEV-25629: Crash in get_sort_by_table() in subquery with order by having outer ref In Item_field::fix_fields(): when the item was resolved to an Item_field in the SELECT's select_list, copy the Item_field's "depended_from" field. Failure to do so caused the item to have incorrect attributes: it pointed to a Field in an upper select but used_tables() didn't return OUTER_REF_TABLE_BIT. --- mysql-test/r/subselect4.result | 10 ++++++++++ mysql-test/t/subselect4.test | 14 ++++++++++++++ sql/item.cc | 1 + 3 files changed, 25 insertions(+) diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result index 4021f717964..2a691799be5 100644 --- a/mysql-test/r/subselect4.result +++ b/mysql-test/r/subselect4.result @@ -2783,3 +2783,13 @@ INSERT INTO t2 VALUES (3),(4); SELECT 1 IN (SELECT (SELECT a FROM t1) AS x FROM t2 GROUP BY x); ERROR 21000: Subquery returns more than 1 row drop table t1,t2; +# +# MDEV-25629: Crash in get_sort_by_table() in subquery with order by having outer ref +# +CREATE TABLE t1 (i1 int); +insert into t1 values (1),(2); +SELECT 1 +FROM (t1 JOIN t1 AS ref_t1 ON +(t1.i1 > (SELECT ref_t1.i1 AS c0 FROM t1 b ORDER BY -c0))); +ERROR 21000: Subquery returns more than 1 row +DROP TABLE t1; diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test index e218e3aab18..58aa7868815 100644 --- a/mysql-test/t/subselect4.test +++ b/mysql-test/t/subselect4.test @@ -2282,3 +2282,17 @@ INSERT INTO t2 VALUES (3),(4); # Optional, fails either way --error ER_SUBQUERY_NO_1_ROW SELECT 1 IN (SELECT (SELECT a FROM t1) AS x FROM t2 GROUP BY x); drop table t1,t2; + +--echo # +--echo # MDEV-25629: Crash in get_sort_by_table() in subquery with order by having outer ref +--echo # +CREATE TABLE t1 (i1 int); +insert into t1 values (1),(2); + +--error ER_SUBQUERY_NO_1_ROW +SELECT 1 +FROM (t1 JOIN t1 AS ref_t1 ON + (t1.i1 > (SELECT ref_t1.i1 AS c0 FROM t1 b ORDER BY -c0))); + +DROP TABLE t1; + diff --git a/sql/item.cc b/sql/item.cc index 42272fe0148..be64edca9a1 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -5513,6 +5513,7 @@ bool Item_field::fix_fields(THD *thd, Item **reference) */ set_max_sum_func_level(thd, select); set_field(new_field); + depended_from= (*((Item_field**)res))->depended_from; return 0; } else

1 0

[Commits] f90be8dab06: MDEV-25078: ALTER INDEX is inconsistent with ADD/DROP/RENAME index
by psergey 14 May '21

14 May '21

revision-id: f90be8dab063dd730a3a8fa3c2ac51ee8549a87e (mariadb-10.6.0-43-gf90be8dab06) parent(s): c67d69abb9b6d05a1c837dc92e1faad770741f55 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-14 15:42:05 +0300 message: MDEV-25078: ALTER INDEX is inconsistent with ADD/DROP/RENAME index Support IF EXISTS in the command that alter index visibility: ALTER TABLE ALTER (KEY|INDEX) [IF EXISTS] index_name [NOT] IGNORED --- mysql-test/main/ignored_index.result | 49 ++++++++++++++++++++++++++++++++++++ mysql-test/main/ignored_index.test | 20 +++++++++++++++ sql/sql_class.h | 6 +++-- sql/sql_table.cc | 28 ++++++++++++++++++++- sql/sql_yacc.yy | 4 +-- 5 files changed, 102 insertions(+), 5 deletions(-) diff --git a/mysql-test/main/ignored_index.result b/mysql-test/main/ignored_index.result index 733e44a3afa..84263dddd4d 100644 --- a/mysql-test/main/ignored_index.result +++ b/mysql-test/main/ignored_index.result @@ -479,3 +479,52 @@ t1 CREATE TABLE `t1` ( KEY `a` (`a`) IGNORED ) ENGINE=MyISAM DEFAULT CHARSET=latin1 DROP TABLE t1; +# +# MDEV-25078, part #2: allow IF EXISTS +# +create table t1 (a int, b int, c int, key(a), key(b), key(c)); +alter table t1 alter key if exists no_such_key ignored; +Warnings: +Note 1176 Key 'no_such_key' doesn't exist in table 't1' +alter table t1 alter key if exists a ignored; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL, + `c` int(11) DEFAULT NULL, + KEY `a` (`a`) IGNORED, + KEY `b` (`b`), + KEY `c` (`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +alter table t1 +alter key if exists no_such_key ignored, +alter key if exists c ignored ; +Warnings: +Note 1176 Key 'no_such_key' doesn't exist in table 't1' +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL, + `c` int(11) DEFAULT NULL, + KEY `a` (`a`) IGNORED, + KEY `b` (`b`), + KEY `c` (`c`) IGNORED +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +alter table t1 +alter key if exists no_such_key not ignored, +alter key if exists c not ignored ; +Warnings: +Note 1176 Key 'no_such_key' doesn't exist in table 't1' +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL, + `c` int(11) DEFAULT NULL, + KEY `a` (`a`) IGNORED, + KEY `b` (`b`), + KEY `c` (`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +drop table t1; diff --git a/mysql-test/main/ignored_index.test b/mysql-test/main/ignored_index.test index a1084f3eb9c..a3d46fe6046 100644 --- a/mysql-test/main/ignored_index.test +++ b/mysql-test/main/ignored_index.test @@ -442,3 +442,23 @@ CREATE TABLE t1 (a INT, KEY (a)); ALTER TABLE t1 ALTER KEY a IGNORED; SHOW CREATE TABLE t1; DROP TABLE t1; + +--echo # +--echo # MDEV-25078, part #2: allow IF EXISTS +--echo # + +create table t1 (a int, b int, c int, key(a), key(b), key(c)); +alter table t1 alter key if exists no_such_key ignored; +alter table t1 alter key if exists a ignored; +show create table t1; +alter table t1 + alter key if exists no_such_key ignored, + alter key if exists c ignored ; +show create table t1; +alter table t1 + alter key if exists no_such_key not ignored, + alter key if exists c not ignored ; +show create table t1; +drop table t1; + + diff --git a/sql/sql_class.h b/sql/sql_class.h index 031fff71ec8..09faeb10ab4 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -386,13 +386,14 @@ class Alter_rename_key : public Sql_alloc class Alter_index_ignorability: public Sql_alloc { public: - Alter_index_ignorability(const char *name, bool is_ignored) : - m_name(name), m_is_ignored(is_ignored) + Alter_index_ignorability(const char *name, bool is_ignored, bool if_exists) : + m_name(name), m_is_ignored(is_ignored), m_if_exists(if_exists) { assert(name != NULL); } const char *name() const { return m_name; } + bool if_exists() const { return m_if_exists; } /* The ignorability after the operation is performed. */ bool is_ignored() const { return m_is_ignored; } @@ -402,6 +403,7 @@ class Alter_index_ignorability: public Sql_alloc private: const char *m_name; bool m_is_ignored; + bool m_if_exists; }; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 209c799dc59..e8fde33191f 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6695,7 +6695,33 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info, rename_key_it.remove(); } } - + /* Handle ALTER KEY IF EXISTS. */ + { + List_iterator<Alter_index_ignorability> ignor_it(alter_info->alter_index_ignorability_list); + Alter_index_ignorability *aii; + while ((aii= ignor_it++)) + { + if (!aii->if_exists()) + continue; + bool exists= false; + for (uint n_key= 0; n_key < table->s->keys; n_key++) + { + if (my_strcasecmp(system_charset_info, aii->name(), + table->key_info[n_key].name.str) == 0) + { + exists= true; + break; + } + } + if (exists) + continue; + push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, + ER_KEY_DOES_NOT_EXISTS, + ER_THD(thd, ER_KEY_DOES_NOT_EXISTS), + aii->name(), table->s->table_name.str); + ignor_it.remove(); + } + } /* ALTER TABLE ADD KEY IF NOT EXISTS */ /* ALTER TABLE ADD FOREIGN KEY IF NOT EXISTS */ { diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 92769dc01f1..c585ff4403c 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -7824,11 +7824,11 @@ alter_list_item: if (unlikely(Lex->add_alter_list($4, $7, $3))) MYSQL_YYABORT; } - | ALTER key_or_index ident ignorability + | ALTER key_or_index opt_if_exists_table_element ident ignorability { LEX *lex= Lex; Alter_index_ignorability *ac= new (thd->mem_root) - Alter_index_ignorability($3.str, $4); + Alter_index_ignorability($4.str, $5, $3); if (ac == NULL) MYSQL_YYABORT; lex->alter_info.alter_index_ignorability_list.push_back(ac);

1 0

[Commits] 4547c6f2833: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ...
by psergey 13 May '21

13 May '21

revision-id: 4547c6f28338afb06a19aab19b4609de14b8a05f (mariadb-10.6.0-46-g4547c6f2833) parent(s): 916c28c9e5ff334f48adff26c74d774a379d96e0 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-13 15:34:25 +0300 message: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ... Mark the JSON_TABLE function as SBR-unsafe. It is not unsafe for the current implementation. But we still mark it as such in order to be future-proof and keep it possible to change JSON data representation in the future. --- mysql-test/suite/json/r/json_table_binlog.result | 26 ++++++++++++++++++++++++ mysql-test/suite/json/t/json_table_binlog.test | 25 +++++++++++++++++++++++ sql/json_table.h | 16 +++++++++++++++ sql/sql_yacc.yy | 2 ++ 4 files changed, 69 insertions(+) diff --git a/mysql-test/suite/json/r/json_table_binlog.result b/mysql-test/suite/json/r/json_table_binlog.result new file mode 100644 index 00000000000..472f7395648 --- /dev/null +++ b/mysql-test/suite/json/r/json_table_binlog.result @@ -0,0 +1,26 @@ +# +# MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +# +create table t1 (a int); +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system function that may return a different value on the slave +set binlog_format='mixed'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +# This must show Annotate_rows, Write_rows_v1 events. Not the statement event +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Annotate_rows # # insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T +master-bin.000001 # Table_map # # table_id: # (test.t1) +master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F +master-bin.000001 # Query # # COMMIT +drop table t1; diff --git a/mysql-test/suite/json/t/json_table_binlog.test b/mysql-test/suite/json/t/json_table_binlog.test new file mode 100644 index 00000000000..dcc05fb855d --- /dev/null +++ b/mysql-test/suite/json/t/json_table_binlog.test @@ -0,0 +1,25 @@ +--source include/have_binlog_format_mixed.inc + +--echo # +--echo # MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +--echo # + +create table t1 (a int); + +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +set binlog_format='mixed'; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); +let $binlog_file= LAST; + +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +--echo # This must show Annotate_rows, Write_rows_v1 events. Not the statement event +--source include/show_binlog_events.inc +drop table t1; diff --git a/sql/json_table.h b/sql/json_table.h index beae5405d25..3560b4ca137 100644 --- a/sql/json_table.h +++ b/sql/json_table.h @@ -183,6 +183,22 @@ class Json_table_column : public Sql_alloc into the TABLE_LIST::table_function. Then the ha_json_table instance is created based on it in the create_table_for_function(). + + == Replication: whether JSON_TABLE is deterministic == + + In sql_yacc.yy, we set BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION whenever + JSON_TABLE is used. The reasoning behind this is as follows: + + In the current MariaDB code, evaluation of JSON_TABLE is deterministic, + that is, for a given input string JSON_TABLE will always produce the same + set of rows in the same order. However one can think of JSON documents + that one can consider indentical which will produce different output. + In order to be feature-proof and withstand changes like: + - sorting JSON object members by name (like MySQL does) + - changing the way duplicate object members are handled + we mark the function as SBR-unsafe. + (If there is ever an issue with this, marking the function as SBR-safe + is a non-intrusive change we will always be able to make) */ class Table_function_json_table : public Sql_alloc diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index fe6112e9592..92769dc01f1 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -11675,6 +11675,8 @@ table_function: new (thd->mem_root) Table_function_json_table($4); if (unlikely(!jt)) MYSQL_YYABORT; + /* See comment for class Table_function_json_table: */ + Lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION); Lex->json_table= jt; Select->parsing_place= NO_MATTER;

1 0

[Commits] a97b533aebb: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ...
by psergey 13 May '21

13 May '21

revision-id: a97b533aebbdbdae72c1ada4161c894de01ec549 (mariadb-10.6.0-42-ga97b533aebb) parent(s): 370b310b1d67ad42df96b75c3876fdcf67a8694f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-13 15:33:57 +0300 message: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ... Mark the JSON_TABLE function as SBR-unsafe. It is not unsafe for the current implementation. But we still mark it as such in order to be future-proof and keep it possible to change JSON data representation in the future. --- mysql-test/suite/json/r/json_table_binlog.result | 26 ++++++++++++++++++++++++ mysql-test/suite/json/t/json_table_binlog.test | 25 +++++++++++++++++++++++ sql/json_table.h | 16 +++++++++++++++ sql/sql_yacc.yy | 2 ++ 4 files changed, 69 insertions(+) diff --git a/mysql-test/suite/json/r/json_table_binlog.result b/mysql-test/suite/json/r/json_table_binlog.result new file mode 100644 index 00000000000..472f7395648 --- /dev/null +++ b/mysql-test/suite/json/r/json_table_binlog.result @@ -0,0 +1,26 @@ +# +# MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +# +create table t1 (a int); +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system function that may return a different value on the slave +set binlog_format='mixed'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +# This must show Annotate_rows, Write_rows_v1 events. Not the statement event +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Annotate_rows # # insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T +master-bin.000001 # Table_map # # table_id: # (test.t1) +master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F +master-bin.000001 # Query # # COMMIT +drop table t1; diff --git a/mysql-test/suite/json/t/json_table_binlog.test b/mysql-test/suite/json/t/json_table_binlog.test new file mode 100644 index 00000000000..dcc05fb855d --- /dev/null +++ b/mysql-test/suite/json/t/json_table_binlog.test @@ -0,0 +1,25 @@ +--source include/have_binlog_format_mixed.inc + +--echo # +--echo # MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +--echo # + +create table t1 (a int); + +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +set binlog_format='mixed'; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); +let $binlog_file= LAST; + +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +--echo # This must show Annotate_rows, Write_rows_v1 events. Not the statement event +--source include/show_binlog_events.inc +drop table t1; diff --git a/sql/json_table.h b/sql/json_table.h index beae5405d25..3560b4ca137 100644 --- a/sql/json_table.h +++ b/sql/json_table.h @@ -183,6 +183,22 @@ class Json_table_column : public Sql_alloc into the TABLE_LIST::table_function. Then the ha_json_table instance is created based on it in the create_table_for_function(). + + == Replication: whether JSON_TABLE is deterministic == + + In sql_yacc.yy, we set BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION whenever + JSON_TABLE is used. The reasoning behind this is as follows: + + In the current MariaDB code, evaluation of JSON_TABLE is deterministic, + that is, for a given input string JSON_TABLE will always produce the same + set of rows in the same order. However one can think of JSON documents + that one can consider indentical which will produce different output. + In order to be feature-proof and withstand changes like: + - sorting JSON object members by name (like MySQL does) + - changing the way duplicate object members are handled + we mark the function as SBR-unsafe. + (If there is ever an issue with this, marking the function as SBR-safe + is a non-intrusive change we will always be able to make) */ class Table_function_json_table : public Sql_alloc diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index fe6112e9592..92769dc01f1 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -11675,6 +11675,8 @@ table_function: new (thd->mem_root) Table_function_json_table($4); if (unlikely(!jt)) MYSQL_YYABORT; + /* See comment for class Table_function_json_table: */ + Lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION); Lex->json_table= jt; Select->parsing_place= NO_MATTER;

1 0

[Commits] c67d69abb9b: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ...
by psergey 13 May '21

13 May '21

revision-id: c67d69abb9b6d05a1c837dc92e1faad770741f55 (mariadb-10.6.0-42-gc67d69abb9b) parent(s): 370b310b1d67ad42df96b75c3876fdcf67a8694f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-05-13 12:25:01 +0300 message: MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe ... Mark the JSON_TABLE function as SBR-unsafe. It is not unsafe for the current implementation. But we still mark it as such in order to be future-proof and keep it possible to change JSON data representation in the future. --- mysql-test/suite/json/r/json_table_binlog.result | 26 ++++++++++++++++++++++++ mysql-test/suite/json/t/json_table_binlog.test | 25 +++++++++++++++++++++++ sql/json_table.h | 16 +++++++++++++++ sql/sql_yacc.yy | 2 ++ 4 files changed, 69 insertions(+) diff --git a/mysql-test/suite/json/r/json_table_binlog.result b/mysql-test/suite/json/r/json_table_binlog.result new file mode 100644 index 00000000000..472f7395648 --- /dev/null +++ b/mysql-test/suite/json/r/json_table_binlog.result @@ -0,0 +1,26 @@ +# +# MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +# +create table t1 (a int); +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +Warnings: +Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system function that may return a different value on the slave +set binlog_format='mixed'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; +# This must show Annotate_rows, Write_rows_v1 events. Not the statement event +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Annotate_rows # # insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T +master-bin.000001 # Table_map # # table_id: # (test.t1) +master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F +master-bin.000001 # Query # # COMMIT +drop table t1; diff --git a/mysql-test/suite/json/t/json_table_binlog.test b/mysql-test/suite/json/t/json_table_binlog.test new file mode 100644 index 00000000000..dcc05fb855d --- /dev/null +++ b/mysql-test/suite/json/t/json_table_binlog.test @@ -0,0 +1,25 @@ +--source include/have_binlog_format_mixed.inc + +--echo # +--echo # MDEV-25154: JSON_TABLE: Queries involving ordinality columns are unsafe for statement binlog and should be marked as such +--echo # + +create table t1 (a int); + +call mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); +set binlog_format='statement'; +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +set binlog_format='mixed'; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); +let $binlog_file= LAST; + +insert into t1 +select * +from json_table('[1,2,3]', '$[*]' columns (a for ordinality)) as T ; + +--echo # This must show Annotate_rows, Write_rows_v1 events. Not the statement event +--source include/show_binlog_events.inc +drop table t1; diff --git a/sql/json_table.h b/sql/json_table.h index beae5405d25..3560b4ca137 100644 --- a/sql/json_table.h +++ b/sql/json_table.h @@ -183,6 +183,22 @@ class Json_table_column : public Sql_alloc into the TABLE_LIST::table_function. Then the ha_json_table instance is created based on it in the create_table_for_function(). + + == Replication: whether JSON_TABLE is deterministic == + + In sql_yacc.yy, we set BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION whenever + JSON_TABLE is used. The reasoning behind this is as follows: + + In the current MariaDB code, evaluation of JSON_TABLE is deterministic, + that is, for a given input string JSON_TABLE will always produce the same + set of rows in the same order. However one can think of JSON documents + that one can consider indentical which will produce different output. + In order to be feature-proof and withstand changes like: + - sorting JSON object members by name (like MySQL does) + - changing the way duplicate object members are handled + we mark the function as SBR-unsafe. + (If there is ever an issue with this, marking the function as SBR-safe + is a non-intrusive change we will always be able to make) */ class Table_function_json_table : public Sql_alloc diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index fe6112e9592..92769dc01f1 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -11675,6 +11675,8 @@ table_function: new (thd->mem_root) Table_function_json_table($4); if (unlikely(!jt)) MYSQL_YYABORT; + /* See comment for class Table_function_json_table: */ + Lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION); Lex->json_table= jt; Select->parsing_place= NO_MATTER;

1 0

[Commits] ec833dd: MDEV-23886 Reusing CTE inside a function fails with table doesn't exist
by IgorBabaev 13 May '21

13 May '21

revision-id: ec833dde82a86cb0bd39900866e88165318e88c0 (mariadb-10.2.31-945-gec833dd) parent(s): 089d82a74be3a96ecaa3e703fb64f1440749d5a6 author: Igor Babaev committer: Igor Babaev timestamp: 2021-05-12 19:32:29 -0700 message: MDEV-23886 Reusing CTE inside a function fails with table doesn't exist In the code existed just before this patch binding of a table reference to the specification of the corresponding CTE happens in the function open_and_process_table(). If the table reference is not the first in the query the specification is cloned in the same way as the specification of a view is cloned for any reference of the view. This works fine for standalone queries, but does not work for stored procedures / functions for the following reason. When the first call of a stored procedure/ function SP is processed the body of SP is parsed. When a query of SP is parsed the info on each encountered table reference is put into a TABLE_LIST object linked into a global chain associated with the query. When parsing of the query is finished the basic info on the table references from this chain except table references to derived tables and information schema tables is put in one hash table associated with SP. When parsing of the body of SP is finished this hash table is used to construct TABLE_LIST objects for all table references mentioned in SP and link them into the list of such objects passed to a pre-locking process that calls open_and_process_table() for each table from the list. When a TABLE_LIST for a view is encountered the view is opened and its specification is parsed. For any table reference occurred in the specification a new TABLE_LIST object is created to be included into the list for pre-locking. After all objects in the pre-locking have been looked through the tables mentioned in the list are locked. Note that the objects referenced CTEs are just skipped here as it is impossible to resolve these references without any info on the context where they occur. Now the statements from the body of SP are executed one by one that. At the very beginning of the execution of a query the tables used in the query are opened and open_and_process_table() now is called for each table reference mentioned in the list of TABLE_LIST objects associated with the query that was built when the query was parsed. For each table reference first the reference is checked against CTEs definitions in whose scope it occurred. If such definition is found the reference is considered resolved and if this is not the first reference to the found CTE the the specification of the CTE is re-parsed and the result of the parsing is added to the parsing tree of the query as a sub-tree. If this sub-tree contains table references to other tables they are added to the list of TABLE_LIST objects associated with the query in order the referenced tables to be opened. When the procedure that opens the tables comes to the TABLE_LIST object created for a non-first reference to a CTE it discovers that the referenced table instance is not locked and reports an error. Thus processing non-first table references to a CTE similar to how references to view are processed does not work for queries used in stored procedures / functions. And the main problem is that the current pre-locking mechanism employed for stored procedures / functions does not allow to save the context in which a CTE reference occur. It's not trivial to save the info about the context where a CTE reference occurs while the resolution of the table reference cannot be done without this context and consequentially the specification for the table reference cannot be determined. This patch solves the above problem by moving resolution of all CTE references at the parsing stage. More exactly references to CTEs occurred in a query are resolved right after parsing of the query has finished. After resolution any CTE reference it is marked as a reference to to derived table. So it is excluded from the hash table created for pre-locking used base tables and view when the first call of a stored procedure / function is processed. This solution required recursive calls of the parser. The function THD::sql_parser() has been added specifically for recursive invocations of the parser. --- mysql-test/r/cte_nonrecursive.result | 213 ++++++++++++++++- mysql-test/r/cte_recursive.result | 8 +- mysql-test/t/cte_nonrecursive.test | 202 ++++++++++++++++ sql/item_subselect.cc | 1 - sql/sp_head.cc | 3 +- sql/sql_base.cc | 33 +-- sql/sql_class.cc | 54 +++++ sql/sql_class.h | 9 +- sql/sql_cte.cc | 437 ++++++++++++++++++++++++----------- sql/sql_cte.h | 84 ++++++- sql/sql_lex.cc | 3 + sql/sql_lex.h | 27 ++- sql/sql_parse.cc | 16 +- sql/sql_prepare.cc | 3 - sql/sql_view.cc | 9 - sql/sql_yacc.yy | 66 +++++- sql/table.h | 37 +++ 17 files changed, 987 insertions(+), 218 deletions(-) diff --git a/mysql-test/r/cte_nonrecursive.result b/mysql-test/r/cte_nonrecursive.result index da954c1..7c6c6e8 100644 --- a/mysql-test/r/cte_nonrecursive.result +++ b/mysql-test/r/cte_nonrecursive.result @@ -606,7 +606,7 @@ with t(c) as (select a from t1 where b >= 'c') select * from t r1 where r1.c=4; show create view v3; View Create View character_set_client collation_connection -v3 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v3` AS with t as (select `test`.`t1`.`a` AS `c` from `test`.`t1` where `test`.`t1`.`b` >= 'c')select `r1`.`c` AS `c` from `t` `r1` where `r1`.`c` = 4 latin1 latin1_swedish_ci +v3 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v3` AS with t(c) as (select `test`.`t1`.`a` AS `c` from `test`.`t1` where `test`.`t1`.`b` >= 'c')select `r1`.`c` AS `c` from `t` `r1` where `r1`.`c` = 4 latin1 latin1_swedish_ci select * from v3; c 4 @@ -618,7 +618,7 @@ with t(c) as (select a from t1 where b >= 'c') select * from t r1, t r2 where r1.c=r2.c and r2.c=4; show create view v4; View Create View character_set_client collation_connection -v4 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v4` AS with t as (select `test`.`t1`.`a` AS `c` from `test`.`t1` where `test`.`t1`.`b` >= 'c')select `r1`.`c` AS `c`,`r2`.`c` AS `d` from (`t` `r1` join `t` `r2`) where `r1`.`c` = `r2`.`c` and `r2`.`c` = 4 latin1 latin1_swedish_ci +v4 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v4` AS with t(c) as (select `test`.`t1`.`a` AS `c` from `test`.`t1` where `test`.`t1`.`b` >= 'c')select `r1`.`c` AS `c`,`r2`.`c` AS `d` from (`t` `r1` join `t` `r2`) where `r1`.`c` = `r2`.`c` and `r2`.`c` = 4 latin1 latin1_swedish_ci select * from v4; c d 4 4 @@ -1120,10 +1120,10 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DERIVED t1 ALL NULL NULL NULL NULL 7 100.00 Using where 5 UNION t1 ALL NULL NULL NULL NULL 7 100.00 Using where NULL UNION RESULT <union2,5> ALL NULL NULL NULL NULL NULL NULL -6 UNION <derived9> ALL NULL NULL NULL NULL 14 100.00 -9 DERIVED t1 ALL NULL NULL NULL NULL 7 100.00 Using where -12 UNION t1 ALL NULL NULL NULL NULL 7 100.00 Using where -NULL UNION RESULT <union9,12> ALL NULL NULL NULL NULL NULL NULL +6 UNION <derived14> ALL NULL NULL NULL NULL 14 100.00 +14 DERIVED t1 ALL NULL NULL NULL NULL 7 100.00 Using where +11 UNION t1 ALL NULL NULL NULL NULL 7 100.00 Using where +NULL UNION RESULT <union14,11> ALL NULL NULL NULL NULL NULL NULL NULL UNION RESULT <union1,6> ALL NULL NULL NULL NULL NULL NULL Warnings: Note 1003 with cte_e as (with cte_o as (with cte_i as (select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 7)select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` > 1)select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 3 and `test`.`t1`.`a` > 1 and `test`.`t1`.`a` < 7 and `test`.`t1`.`a` > 1 union select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` > 4 and `test`.`t1`.`a` > 1 and `test`.`t1`.`a` < 7 and `test`.`t1`.`a` > 1)select `cte_e1`.`a` AS `a` from `cte_e` `cte_e1` where `cte_e1`.`a` > 1 union select `cte_e2`.`a` AS `a` from `cte_e` `cte_e2` @@ -1763,4 +1763,205 @@ a c 2 1 7 3 drop table t1; +# +# MDEV-23886: Stored Function returning the result of a query +# that uses CTE over a table twice +# +create table t1 (c1 int); +insert into t1 values (1),(2),(6); +create function f1() returns int return +( with cte1 as (select c1 from t1) +select sum(c1) from +(select * from cte1 union all select * from cte1) dt +); +select f1(); +f1() +18 +create function f2() returns int return +( with cte1 as (select c1 from t1) +select sum(s.c1) from cte1 as s, cte1 as t where s.c1=t.c1 +); +select f2(); +f2() +9 +create function f3() returns int return +( with cte1 as (select c1 from t1) +select +case +when exists(select 1 from cte1 where c1 between 1 and 2) then 1 +when exists(select 1 from cte1 where c1 between 5 and 6) then 2 +else 0 +end +); +select f3(); +f3() +1 +create view v1 as (select c1 from t1); +create function f4() returns int return +( select sum(c1) from +(select * from v1 union all select * from v1) dt +); +select f4(); +f4() +18 +create function f5() returns int return +( select sum(s.c1) from v1 as s, v1 as t where s.c1=t.c1 +); +select f5(); +f5() +9 +create view v2(s) as +with cte1 as (select c1 from t1) +select sum(c1) from (select * from cte1 union all select * from cte1) dt; +create function f6() returns int return +(select s from v2); +select f6(); +f6() +18 +create function f7() returns int return +( select r.s from v2 as r, v2 as t where r.s=t.s +); +select f7(); +f7() +18 +select f5() + f6(); +f5() + f6() +27 +prepare stmt from "select f5() + f6();"; +execute stmt; +f5() + f6() +27 +execute stmt; +f5() + f6() +27 +deallocate prepare stmt; +drop function f1; +drop function f2; +drop function f3; +drop function f4; +drop function f5; +drop function f6; +drop function f7; +drop view v1; +drop view v2; +create table t2 (a int, b int); +insert into t2 +with cte1 as (select c1 from t1) +select * from cte1 as s, cte1 as t where s.c1=t.c1 and s.c1 > 5; +select * from t2; +a b +6 6 +create procedure p1() +begin +insert into t2 +with cte1 as (select c1 from t1) +select * from cte1 as s, cte1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | +call p1(); +select * from t2; +a b +6 6 +2 2 +drop procedure p1; +# checking CTE resolution for queries with hanging CTEs +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from cte3; +a b +1 2 +select * from t2; +a b +6 6 +2 2 +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from t2; +a b +6 6 +2 2 +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where c1 >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from t2; +ERROR 42S22: Unknown column 'c1' in 'where clause' +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.c1) +select * from t2; +ERROR 42S22: Unknown column 'cte2.c1' in 'where clause' +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select * from cte2; +a b +1 1 +2 2 +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select * from t2; +a b +6 6 +2 2 +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=c1) +select * from t2; +ERROR 23000: Column 'c1' in where clause is ambiguous +with cte3 as +( with cte2(a,b) as +( with cte1 as (select * from t1 where c1 <= 2) +select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select r1.a,r2.b from cte2 as r1, cte2 as r2) +select * from cte3; +a b +1 1 +2 1 +1 2 +2 2 +with cte3 as +( with cte2(a,b) as +( with cte1 as (select * from t1 where c1 <= 2) +select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select r1.a,r2.b from cte2 as r1, cte2 as r2) +select * from t2; +a b +6 6 +2 2 +with cte3 as +( with cte2(a,b) as +( with cte1 as (select * from t1 where c1 <= 2) +select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select r1.c1,r2.c1 from cte2 as r1, cte2 as r2) +select * from t2; +ERROR 42S22: Unknown column 'r1.c1' in 'field list' +create procedure p1() +begin +insert into t2 +with cte1 as (select c1 from t1) +select * from t1 as s, t1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | +call p1(); +select * from t2; +a b +6 6 +2 2 +2 2 +drop procedure p1; +create procedure p1() +begin +insert into t2 +with cte1 as (select a from t1) +select * from t1 as s, t1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | +call p1(); +ERROR 42S22: Unknown column 'a' in 'field list' +drop procedure p1; +drop table t1,t2; # End of 10.2 tests diff --git a/mysql-test/r/cte_recursive.result b/mysql-test/r/cte_recursive.result index b6b4ed7..3e92652 100644 --- a/mysql-test/r/cte_recursive.result +++ b/mysql-test/r/cte_recursive.result @@ -699,7 +699,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra NULL UNION RESULT <union3,4,5> ALL NULL NULL NULL NULL NULL NULL 2 DERIVED <derived3> ALL NULL NULL NULL NULL 12 100.00 Using where Warnings: -Note 1003 with recursive ancestor_couple_ids as (select `a`.`father` AS `h_id`,`a`.`mother` AS `w_id` from `coupled_ancestors` `a` where `a`.`father` is not null and `a`.`mother` is not null), coupled_ancestors as (select `test`.`folks`.`id` AS `id`,`test`.`folks`.`name` AS `name`,`test`.`folks`.`dob` AS `dob`,`test`.`folks`.`father` AS `father`,`test`.`folks`.`mother` AS `mother` from `test`.`folks` where `test`.`folks`.`name` = 'Me' union all select `test`.`p`.`id` AS `id`,`test`.`p`.`name` AS `name`,`test`.`p`.`dob` AS `dob`,`test`.`p`.`father` AS `father`,`test`.`p`.`mother` AS `mother` from `test`.`folks` `p` join `ancestor_couple_ids` `fa` where `test`.`p`.`id` = `fa`.`h_id` union all select `test`.`p`.`id` AS `id`,`test`.`p`.`name` AS `name`,`test`.`p`.`dob` AS `dob`,`test`.`p`.`father` AS `father`,`test`.`p`.`mother` AS `mother` from `test`.`folks` `p` join `ancestor_couple_ids` `ma` where `test`.`p`.`id` = `ma`.`w_id`)select `h`.`name` AS `name`,`h`.`dob` AS `dob`,`w`.`name ` AS `name`,`w`.`dob` AS `dob` from `ancestor_couple_ids` `c` join `coupled_ancestors` `h` join `coupled_ancestors` `w` where `h`.`id` = `c`.`h_id` and `w`.`id` = `c`.`w_id` +Note 1003 with recursive ancestor_couple_ids(h_id,w_id) as (select `a`.`father` AS `h_id`,`a`.`mother` AS `w_id` from `coupled_ancestors` `a` where `a`.`father` is not null and `a`.`mother` is not null), coupled_ancestors(id,name,dob,father,mother) as (select `test`.`folks`.`id` AS `id`,`test`.`folks`.`name` AS `name`,`test`.`folks`.`dob` AS `dob`,`test`.`folks`.`father` AS `father`,`test`.`folks`.`mother` AS `mother` from `test`.`folks` where `test`.`folks`.`name` = 'Me' union all select `test`.`p`.`id` AS `id`,`test`.`p`.`name` AS `name`,`test`.`p`.`dob` AS `dob`,`test`.`p`.`father` AS `father`,`test`.`p`.`mother` AS `mother` from `test`.`folks` `p` join `ancestor_couple_ids` `fa` where `test`.`p`.`id` = `fa`.`h_id` union all select `test`.`p`.`id` AS `id`,`test`.`p`.`name` AS `name`,`test`.`p`.`dob` AS `dob`,`test`.`p`.`father` AS `father`,`test`.`p`.`mother` AS `mother` from `test`.`folks` `p` join `ancestor_couple_ids` `ma` where `test`.`p`.`id` = `ma`.`w_id`)select `h`.`name` AS `name`,`h`.`dob` AS `dob`,`w`.`name` AS `name`,`w`.`dob` AS `dob` from `ancestor_couple_ids` `c` join `coupled_ancestors` `h` join `coupled_ancestors` `w` where `h`.`id` = `c`.`h_id` and `w`.`id` = `c`.`w_id` # simple mutual recursion with recursive ancestor_couple_ids(h_id, w_id) @@ -3047,7 +3047,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 4 DEPENDENT SUBQUERY <derived2> ALL NULL NULL NULL NULL 16 100.00 Using where NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL Warnings: -Note 1003 with recursive destinations as (select `test`.`a`.`arrival` AS `city`,1 AS `legs` from `test`.`flights` `a` where `test`.`a`.`departure` = 'Cairo' union select `test`.`b`.`arrival` AS `arrival`,`r`.`legs` + 1 AS `r.legs + 1` from `destinations` `r` join `test`.`flights` `b` where `r`.`city` = `test`.`b`.`departure` and !<in_optimizer>(`test`.`b`.`arrival`,<exists>(select `destinations`.`city` from `destinations` where trigcond(`test`.`b`.`arrival` = `destinations`.`city` or `destinations`.`city` is null) having trigcond(`destinations`.`city` is null))))select `destinations`.`city` AS `city`,`destinations`.`legs` AS `legs` from `destinations` +Note 1003 with recursive destinations(city,legs) as (select `test`.`a`.`arrival` AS `city`,1 AS `legs` from `test`.`flights` `a` where `test`.`a`.`departure` = 'Cairo' union select `test`.`b`.`arrival` AS `arrival`,`r`.`legs` + 1 AS `r.legs + 1` from `destinations` `r` join `test`.`flights` `b` where `r`.`city` = `test`.`b`.`departure` and !<in_optimizer>(`test`.`b`.`arrival`,<exists>(select `destinations`.`city` from `destinations` where trigcond(`test`.`b`.`arrival` = `destinations`.`city` or `destinations`.`city` is null) having trigcond(`destinations`.`city` is null))))select `destinations`.`city` AS `city`,`destinations`.`legs` AS `legs` from `destinations` set standard_compliant_cte=default; drop table flights; # @@ -3334,7 +3334,7 @@ NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL 5 DERIVED <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where 5 DERIVED t1 ALL NULL NULL NULL NULL 3 100.00 Using where; Using join buffer (flat, BNL join) Warnings: -Note 1003 with recursive rcte as (select 1 AS `a` union select cast(`rcte`.`a` + 1 as unsigned) AS `cast(a+1 as unsigned)` from `rcte` where `rcte`.`a` < 10), cte1 as (select count(0) AS `c1` from `rcte` join `test`.`t1` where `rcte`.`a` between 3 and 5 and `test`.`t1`.`id` = `rcte`.`a` - 3), cte2 as (select count(0) AS `c2` from `rcte` join `test`.`t1` where `rcte`.`a` between 7 and 8 and `test`.`t1`.`id` = `rcte`.`a` - 7)select `cte1`.`c1` AS `c1`,`cte2`.`c2` AS `c2` from `cte1` join `cte2` +Note 1003 with recursive rcte(a) as (select 1 AS `a` union select cast(`rcte`.`a` + 1 as unsigned) AS `cast(a+1 as unsigned)` from `rcte` where `rcte`.`a` < 10), cte1 as (select count(0) AS `c1` from `rcte` join `test`.`t1` where `rcte`.`a` between 3 and 5 and `test`.`t1`.`id` = `rcte`.`a` - 3), cte2 as (select count(0) AS `c2` from `rcte` join `test`.`t1` where `rcte`.`a` between 7 and 8 and `test`.`t1`.`id` = `rcte`.`a` - 7)select `cte1`.`c1` AS `c1`,`cte2`.`c2` AS `c2` from `cte1` join `cte2` prepare stmt from "with recursive rcte(a) as (select 1 union select cast(a+1 as unsigned) from rcte where a < 10), @@ -3420,7 +3420,7 @@ NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL 4 DERIVED <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where 4 DERIVED t1 ALL NULL NULL NULL NULL 3 100.00 Using where; Using join buffer (flat, BNL join) Warnings: -Note 1003 with recursive rcte as (select 1 AS `a` union select cast(`rcte`.`a` + 1 as unsigned) AS `cast(a+1 as unsigned)` from `rcte` where `rcte`.`a` < 10), cte1 as (select count(0) AS `c1` from `rcte` join `test`.`t1` where `rcte`.`a` between 3 and 5 and `test`.`t1`.`id` = `rcte`.`a` - 3), cte2 as (select count(0) AS `c2` from `rcte` join `test`.`t1` where `rcte`.`a` between 7 and 8 and `test`.`t1`.`id` = `rcte`.`a` - 7)select `cte2`.`c2` AS `c2`,`cte1`.`c1` AS `c1` from `cte2` join `cte1` +Note 1003 with recursive rcte(a) as (select 1 AS `a` union select cast(`rcte`.`a` + 1 as unsigned) AS `cast(a+1 as unsigned)` from `rcte` where `rcte`.`a` < 10), cte1 as (select count(0) AS `c1` from `rcte` join `test`.`t1` where `rcte`.`a` between 3 and 5 and `test`.`t1`.`id` = `rcte`.`a` - 3), cte2 as (select count(0) AS `c2` from `rcte` join `test`.`t1` where `rcte`.`a` between 7 and 8 and `test`.`t1`.`id` = `rcte`.`a` - 7)select `cte2`.`c2` AS `c2`,`cte1`.`c1` AS `c1` from `cte2` join `cte1` prepare stmt from "with recursive rcte(a) as (select 1 union select cast(a+1 as unsigned) from rcte where a < 10), diff --git a/mysql-test/t/cte_nonrecursive.test b/mysql-test/t/cte_nonrecursive.test index c2ebc92..f994781 100644 --- a/mysql-test/t/cte_nonrecursive.test +++ b/mysql-test/t/cte_nonrecursive.test @@ -1261,4 +1261,206 @@ select a, c from cte as r2 where a > 4; drop table t1; +--echo # +--echo # MDEV-23886: Stored Function returning the result of a query +--echo # that uses CTE over a table twice +--echo # + +create table t1 (c1 int); +insert into t1 values (1),(2),(6); + +create function f1() returns int return +( with cte1 as (select c1 from t1) + select sum(c1) from + (select * from cte1 union all select * from cte1) dt +); +select f1(); + +create function f2() returns int return +( with cte1 as (select c1 from t1) + select sum(s.c1) from cte1 as s, cte1 as t where s.c1=t.c1 +); +select f2(); + +create function f3() returns int return +( with cte1 as (select c1 from t1) + select + case + when exists(select 1 from cte1 where c1 between 1 and 2) then 1 + when exists(select 1 from cte1 where c1 between 5 and 6) then 2 + else 0 + end +); +select f3(); + +create view v1 as (select c1 from t1); + +create function f4() returns int return +( select sum(c1) from + (select * from v1 union all select * from v1) dt +); +select f4(); + +create function f5() returns int return +( select sum(s.c1) from v1 as s, v1 as t where s.c1=t.c1 +); +select f5(); + +create view v2(s) as +with cte1 as (select c1 from t1) +select sum(c1) from (select * from cte1 union all select * from cte1) dt; + +create function f6() returns int return +(select s from v2); +select f6(); + +create function f7() returns int return +( select r.s from v2 as r, v2 as t where r.s=t.s +); +select f7(); + +select f5() + f6(); + +prepare stmt from "select f5() + f6();"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +drop function f1; +drop function f2; +drop function f3; +drop function f4; +drop function f5; +drop function f6; +drop function f7; + +drop view v1; +drop view v2; + +create table t2 (a int, b int); + +insert into t2 +with cte1 as (select c1 from t1) +select * from cte1 as s, cte1 as t where s.c1=t.c1 and s.c1 > 5; + +select * from t2; + +delimiter |; + +create procedure p1() +begin +insert into t2 +with cte1 as (select c1 from t1) +select * from cte1 as s, cte1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | + +delimiter ;| + +call p1(); +select * from t2; + +drop procedure p1; + +--echo # checking CTE resolution for queries with hanging CTEs + +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from cte3; + +select * from t2; + +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from t2; + +--error ER_BAD_FIELD_ERROR +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where c1 >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.b) +select * from t2; + +--error ER_BAD_FIELD_ERROR +with +cte1(a) as (select * from t1 where c1 <= 2), +cte2(b) as (select * from cte1 where a >= 2), +cte3 as (select * from cte1,cte2 where cte1.a < cte2.c1) +select * from t2; + +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select * from cte2; + +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) +select * from t2; + +--error ER_NON_UNIQ_ERROR +with +cte1 as (select * from t1 where c1 <= 2), +cte2(a,b) as (select * from cte1 as s1, cte1 as s2 where s1.c1=c1) +select * from t2; + +with cte3 as +( with cte2(a,b) as + ( with cte1 as (select * from t1 where c1 <= 2) + select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) + select r1.a,r2.b from cte2 as r1, cte2 as r2) +select * from cte3; + +with cte3 as +( with cte2(a,b) as + ( with cte1 as (select * from t1 where c1 <= 2) + select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) + select r1.a,r2.b from cte2 as r1, cte2 as r2) +select * from t2; + +--error ER_BAD_FIELD_ERROR +with cte3 as +( with cte2(a,b) as + ( with cte1 as (select * from t1 where c1 <= 2) + select * from cte1 as s1, cte1 as s2 where s1.c1=s2.c1) + select r1.c1,r2.c1 from cte2 as r1, cte2 as r2) +select * from t2; + +delimiter |; + +create procedure p1() +begin +insert into t2 +with cte1 as (select c1 from t1) +select * from t1 as s, t1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | + +delimiter ;| + +call p1(); +select * from t2; + +drop procedure p1; + +delimiter |; + +create procedure p1() +begin +insert into t2 +with cte1 as (select a from t1) +select * from t1 as s, t1 as t where s.c1=t.c1 and s.c1 <= 2 and t.c1 >= 2; +end | + +delimiter ;| + +--error ER_BAD_FIELD_ERROR +call p1(); + +drop procedure p1; + +drop table t1,t2; + --echo # End of 10.2 tests diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index ed8e5e9..21021f14 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -1774,7 +1774,6 @@ double Item_in_subselect::val_real() As far as Item_in_subselect called only from Item_in_optimizer this method should not be used */ - DBUG_ASSERT(0); DBUG_ASSERT(fixed == 1); if (forced_const) return value; diff --git a/sql/sp_head.cc b/sql/sp_head.cc index b205d25..6a6bdf8 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3087,8 +3087,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, #endif if (open_tables) - res= check_dependencies_in_with_clauses(m_lex->with_clauses_list) || - instr->exec_open_and_lock_tables(thd, m_lex->query_tables); + res= instr->exec_open_and_lock_tables(thd, m_lex->query_tables); if (!res) { diff --git a/sql/sql_base.cc b/sql/sql_base.cc index e3c30c7..c9a3422 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3406,7 +3406,11 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags, if (tables->derived) { if (!tables->view) + { + if (!tables->is_derived()) + tables->set_derived(); goto end; + } /* We restore view's name and database wiped out by derived tables processing and fall back to standard open process in order to @@ -3418,35 +3422,6 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags, tables->table_name= tables->view_name.str; tables->table_name_length= tables->view_name.length; } - else if (tables->select_lex) - { - /* - Check whether 'tables' refers to a table defined in a with clause. - If so set the reference to the definition in tables->with. - */ - if (!tables->with) - tables->with= tables->select_lex->find_table_def_in_with_clauses(tables); - /* - If 'tables' is defined in a with clause set the pointer to the - specification from its definition in tables->derived. - */ - if (tables->with) - { - if (tables->is_recursive_with_table() && - !tables->is_with_table_recursive_reference()) - { - tables->with->rec_outer_references++; - With_element *with_elem= tables->with; - while ((with_elem= with_elem->get_next_mutually_recursive()) != - tables->with) - with_elem->rec_outer_references++; - } - if (tables->set_as_with_table(thd, tables->with)) - DBUG_RETURN(1); - else - goto end; - } - } if (!tables->derived && is_infoschema_db(tables->db, tables->db_length)) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index ac9df47..80ff0aa 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -2525,6 +2525,60 @@ void THD::close_active_vio() #endif +/* + @brief MySQL parser used for recursive invocations + + @param old_lex The LEX structure in the state when this parser + is called recursively + @param lex The LEX structure used to parse a new SQL fragment + @param str The SQL fragment to parse + @param str_len The length of the SQL fragment to parse + @param stmt_prepare_mode true <=> when parsing a prepare statement + + @details + This function is to be used when parsing of an SQL fragment is + needed within one of the grammar rules. + + @notes + Currently the function is used only when the specification of a CTE + is parsed for the not first and not recursive references of the CTE. + + @retval false On a successful parsing of the fragment + @retval true Otherwise +*/ + +bool THD::sql_parser(LEX *old_lex, LEX *lex, + char *str, uint str_len, bool stmt_prepare_mode) +{ + extern int MYSQLparse(THD * thd); + + bool parse_status= false; + Parser_state parser_state; + Parser_state *old_parser_state= m_parser_state; + + if (parser_state.init(this, str, str_len)) + return true; + + m_parser_state= &parser_state; + parser_state.m_lip.stmt_prepare_mode= stmt_prepare_mode; + parser_state.m_lip.multi_statements= false; + parser_state.m_lip.m_digest= NULL; + + lex->param_list= old_lex->param_list; + lex->sphead= old_lex->sphead; + lex->spname= old_lex->spname; + lex->spcont= old_lex->spcont; + lex->sp_chistics= old_lex->sp_chistics; + lex->trg_chistics= old_lex->trg_chistics; + + parse_status= MYSQLparse(this) != 0; + + m_parser_state= old_parser_state; + + return parse_status; +} + + struct Item_change_record: public ilink { Item **place; diff --git a/sql/sql_class.h b/sql/sql_class.h index e08bb3e..7c546b6 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4002,14 +4002,11 @@ class THD :public Statement, to resolve all CTE names as we don't need this message to be thrown for any CTE references. */ - if (!lex->with_clauses_list) + if (!lex->with_cte_resolution) { my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); return TRUE; } - /* This will allow to throw an error later for non-CTE references */ - *p_db= NULL; - *p_db_length= 0; } else { @@ -4544,6 +4541,10 @@ class THD :public Statement, current_linfo= 0; mysql_mutex_unlock(&LOCK_thread_count); } + + bool sql_parser(LEX *old_lex, LEX *lex, + char *str, uint str_len, bool stmt_prepare_mode); + }; inline void add_to_active_threads(THD *thd) diff --git a/sql/sql_cte.cc b/sql/sql_cte.cc index 3a2301a..9dad33d 100644 --- a/sql/sql_cte.cc +++ b/sql/sql_cte.cc @@ -58,7 +58,7 @@ bool With_clause::add_with_element(With_element *elem) true on failure */ -bool check_dependencies_in_with_clauses(With_clause *with_clauses_list) +bool LEX::check_dependencies_in_with_clauses() { for (With_clause *with_clause= with_clauses_list; with_clause; @@ -76,6 +76,200 @@ bool check_dependencies_in_with_clauses(With_clause *with_clauses_list) /** @brief + Resolve references to CTE in specification of hanging CTE + + @details + A CTE to which there are no references in the query is called hanging CTE. + Although such CTE is not used for execution its specification must be + subject to context analysis. All errors concerning references to + non-existing tables or fields occurred in the specification must be + reported as well as all other errors caught at the prepare stage. + The specification of a hanging CTE might contain references to other + CTE outside of the specification and within it if the specification + contains a with clause. This function resolves all such references for + all hanging CTEs encountered in the processed query. + + @retval + false on success + true on failure +*/ + +bool +LEX::resolve_references_to_cte_in_hanging_cte() +{ + for (With_clause *with_clause= with_clauses_list; + with_clause; with_clause= with_clause->next_with_clause) + { + for (With_element *with_elem= with_clause->with_list.first; + with_elem; with_elem= with_elem->next) + { + if (!with_elem->is_referenced()) + { + TABLE_LIST *first_tbl= + with_elem->spec->first_select()->table_list.first; + TABLE_LIST **with_elem_end_pos= with_elem->head->tables_pos.end_pos; + if (first_tbl && resolve_references_to_cte(first_tbl, with_elem_end_pos)) + return true; + } + } + } + return false; +} + + +/** + @brief + Resolve table references to CTE from a sub-chain of table references + + @param tables Points to the beginning of the sub-chain + @param tables_last Points to the address with the sub-chain barrier + + @details + The method resolves tables references to CTE from the chain of + table references specified by the parameters 'tables' and 'tables_last'. + It resolves the references against the CTE definition occurred in a query + or the specification of a CTE whose parsing tree is represented by + this LEX structure. The method is always called right after the process + of parsing the query or of the specification of a CTE has been finished, + thus the chain of table references used in the parsed fragment has been + already built. It is assumed that parameters of the method specify a + a sub-chain of this chain. + If a table reference can be potentially a table reference to a CTE and it + has not been resolved yet then the method tries to find the definition + of the CTE against which the reference can be resolved. If it succeeds + it sets the field TABLE_LIST::with to point to the found definition. + It also sets the field TABLE_LIST::derived to point to the specification + of the found CTE and sets TABLE::db.str to empty_c_string. This will + allow to handle this table reference like a reference to a derived handle. + If another table reference has been already resolved against this CTE + and this CTE is not recursive then a clone of the CTE specification is + constructed using the function With_element::clone_parsed_spec() and + TABLE_LIST::derived is set to point to this clone rather than to the + original specification. + If the method does not find a matched CTE definition in the parsed fragment + then in the case when the flag this->only_cte_resolution is set to true + it just moves to the resolution of the next table reference from the + specified sub-chain while in the case when this->only_cte_resolution is set + to false the method additionally sets an mdl request for this table + reference. + + @notes + The flag this->only_cte_resolution is set to true in the cases when + the failure to resolve a table reference as a CTE reference within + the fragment associated with this LEX structure does not imply that + this table reference cannot be resolved as such at all. + + @retval false On success: no errors reported, no memory allocations failed + @retval true Otherwise +*/ + +bool LEX::resolve_references_to_cte(TABLE_LIST *tables, + TABLE_LIST **tables_last) +{ + With_element *with_elem= 0; + + for (TABLE_LIST *tbl= tables; tbl != *tables_last; tbl= tbl->next_global) + { + if (tbl->derived) + continue; + if (!tbl->db && !tbl->with) + tbl->with= tbl->select_lex->find_table_def_in_with_clauses(tbl); + if (!tbl->with) // no CTE matches table reference tbl + { + if (only_cte_resolution) + continue; + if (!tbl->db) // no database specified in table reference tbl + { + if (!thd->db) // no default database is set + { + my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); + return true; + } + if (copy_db_to(&tbl->db, &tbl->db_length)) + return true; + if (!(tbl->table_options & TL_OPTION_ALIAS)) + tbl->mdl_request.init(MDL_key::TABLE, tbl->db, + tbl->table_name, + tbl->mdl_type, MDL_TRANSACTION); + tbl->mdl_request.set_type((tbl->lock_type >= TL_WRITE_ALLOW_WRITE) ? + MDL_SHARED_WRITE : MDL_SHARED_READ); + } + continue; + } + with_elem= tbl->with; + if (tbl->is_recursive_with_table() && + !tbl->is_with_table_recursive_reference()) + { + tbl->with->rec_outer_references++; + while ((with_elem= with_elem->get_next_mutually_recursive()) != + tbl->with) + with_elem->rec_outer_references++; + } + if (!with_elem->is_used_in_query || with_elem->is_recursive) + { + tbl->derived= with_elem->spec; + if (tbl->derived != tbl->select_lex->master_unit() && + !with_elem->is_recursive && + !tbl->is_with_table_recursive_reference()) + { + tbl->derived->move_as_slave(tbl->select_lex); + } + with_elem->is_used_in_query= true; + } + else + { + if (!(tbl->derived= tbl->with->clone_parsed_spec(thd->lex, tbl))) + return true; + } + tbl->db= empty_c_string; + tbl->db_length= 0; + tbl->schema_table= 0; + if (tbl->derived) + { + tbl->derived->first_select()->linkage= DERIVED_TABLE_TYPE; + } + if (tbl->with->is_recursive && tbl->is_with_table_recursive_reference()) + continue; + with_elem->inc_references(); + } + return false; +} + + +/** + @brief + Find out dependencies between CTEs, resolve references to them + + @details + The function can be called in two modes. With this->with_cte_resolution + set to false the function only finds out all dependencies between CTEs + used in a query expression with a WITH clause whose parsing has been + just finished. Based on these dependencies recursive CTEs are detected. + If this->with_cte_resolution is set to true the function additionally + resolves all references to CTE occurred in this query expression. + + @retval + true on failure + false on success +*/ + +bool +LEX::check_cte_dependencies_and_resolve_references() +{ + if (check_dependencies_in_with_clauses()) + return true; + if (!with_cte_resolution) + return false; + if (resolve_references_to_cte(query_tables, query_tables_last)) + return true; + if (resolve_references_to_cte_in_hanging_cte()) + return true; + return false; +} + + +/** + @brief Check dependencies between tables defined in this with clause @details @@ -112,10 +306,11 @@ bool With_clause::check_dependencies() elem != with_elem; elem= elem->next) { - if (my_strcasecmp(system_charset_info, with_elem->query_name->str, - elem->query_name->str) == 0) + if (my_strcasecmp(system_charset_info, with_elem->get_name_str(), + elem->get_name_str()) == 0) { - my_error(ER_DUP_QUERY_NAME, MYF(0), with_elem->query_name->str); + my_error(ER_DUP_QUERY_NAME, MYF(0), + with_elem->get_name_str()); return true; } } @@ -222,12 +417,12 @@ With_element *With_clause::find_table_def(TABLE_LIST *table, with_elem != barrier; with_elem= with_elem->next) { - if (my_strcasecmp(system_charset_info, with_elem->query_name->str, - table->table_name) == 0 && + if (my_strcasecmp(system_charset_info, with_elem->get_name_str(), + table->table_name) == 0 && !table->is_fqtn) { table->set_derived(); - table->db= empty_c_string; + with_elem->referenced= true; return with_elem; } } @@ -584,7 +779,7 @@ bool With_clause::check_anchors() if (elem == with_elem) { my_error(ER_RECURSIVE_WITHOUT_ANCHORS, MYF(0), - with_elem->query_name->str); + with_elem->get_name_str()); return true; } } @@ -617,7 +812,7 @@ bool With_clause::check_anchors() if (elem->work_dep_map & elem->get_elem_map()) { my_error(ER_UNACCEPTABLE_MUTUAL_RECURSION, MYF(0), - with_elem->query_name->str); + with_elem->get_name_str()); return true; } } @@ -771,7 +966,8 @@ bool With_element::set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, @brief Create a clone of the specification for the given with table - @param thd The context of the statement containing this with element + @param old_lex The LEX structure created for the query or CTE specification + where this With_element is defined @param with_table The reference to the table defined in this element for which the clone is created. @@ -781,12 +977,13 @@ bool With_element::set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, this element. The clone is created when the string with the specification saved in unparsed_spec is fed into the parser as an input string. The parsing - this string a unit object representing the specification is build. + this string a unit object representing the specification is built. A chain of all table references occurred in the specification is also formed. The method includes the new unit and its sub-unit into hierarchy of the units of the main query. I also insert the constructed chain of the table references into the chain of all table references of the main query. + The method resolves all references to CTE in the clone. @note Clones is created only for not first references to tables defined in @@ -802,115 +999,129 @@ bool With_element::set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, NULL - otherwise */ -st_select_lex_unit *With_element::clone_parsed_spec(THD *thd, +st_select_lex_unit *With_element::clone_parsed_spec(LEX *old_lex, TABLE_LIST *with_table) { + THD *thd= old_lex->thd; LEX *lex; - st_select_lex_unit *res= NULL; - Query_arena backup; - Query_arena *arena= thd->activate_stmt_arena_if_needed(&backup); + st_select_lex_unit *res= NULL; if (!(lex= (LEX*) new(thd->mem_root) st_lex_local)) - { - if (arena) - thd->restore_active_arena(arena, &backup); return res; - } - LEX *old_lex= thd->lex; thd->lex= lex; bool parse_status= false; - Parser_state parser_state; - TABLE_LIST *spec_tables; - TABLE_LIST *spec_tables_tail; st_select_lex *with_select; char save_end= unparsed_spec.str[unparsed_spec.length]; unparsed_spec.str[unparsed_spec.length]= '\0'; - if (parser_state.init(thd, unparsed_spec.str, unparsed_spec.length)) - goto err; - parser_state.m_lip.stmt_prepare_mode= stmt_prepare_mode; - parser_state.m_lip.multi_statements= false; - parser_state.m_lip.m_digest= NULL; lex_start(thd); lex->clone_spec_offset= unparsed_spec_offset; - lex->param_list= old_lex->param_list; - lex->sphead= old_lex->sphead; - lex->spname= old_lex->spname; - lex->spcont= old_lex->spcont; - lex->sp_chistics= old_lex->sp_chistics; - - lex->stmt_lex= old_lex; - with_select= &lex->select_lex; - with_select->select_number= ++thd->lex->stmt_lex->current_select_number; - parse_status= parse_sql(thd, &parser_state, 0); + lex->with_cte_resolution= true; + + /* + The specification of a CTE is to be parsed as a regular query. + At the very end of the parsing query the function + check_cte_dependencies_and_resolve_references() will be called. + It will check the dependencies between CTEs that are defined + within the query and will resolve CTE references in this query. + If a table reference is not resolved as a CTE reference within + this query it still can be resolved as a reference to a CTE defined + in the same clause as the CTE whose specification is to be parsed + or defined in an embedding CTE definition. + + Example: + with + cte1 as ( ... ), + cte2 as ([WITH ...] select ... from cte1 ...) + select ... from cte2 as r, ..., cte2 as s ... + + Here the specification of cte2 has be cloned for table reference + with alias s1. The specification contains a reference to cte1 + that is defined outside this specification. If the reference to + cte1 cannot be resolved within the specification of cte2 it's + not necessarily has to be a reference to a non-CTE table. That's + why the flag lex->only_cte_resolution has to be set to true + before parsing of the specification of cte2 invoked by this + function starts. Otherwise an mdl_lock would be requested for s + and this would not be correct. + */ + + lex->only_cte_resolution= true; + + lex->stmt_lex= old_lex->stmt_lex ? old_lex->stmt_lex : old_lex; + + parse_status= thd->sql_parser(old_lex, lex, + (char*) unparsed_spec.str, + (unsigned int)unparsed_spec.length, + stmt_prepare_mode); + unparsed_spec.str[unparsed_spec.length]= save_end; + with_select= lex->unit.first_select(); + with_select->select_number= ++lex->stmt_lex->current_select_number; if (parse_status) goto err; - if (check_dependencies_in_with_clauses(lex->with_clauses_list)) - goto err; - - spec_tables= lex->query_tables; - spec_tables_tail= 0; - for (TABLE_LIST *tbl= spec_tables; - tbl; - tbl= tbl->next_global) - { - if (!tbl->derived && !tbl->schema_table && - thd->open_temporary_table(tbl)) - goto err; - spec_tables_tail= tbl; - } - if (spec_tables) + /* + The global chain of TABLE_LIST objects created for the specification that + just has been parsed is added to such chain that contains the reference + to the CTE whose specification is parsed right after the TABLE_LIST object + created for the reference. + */ + if (lex->query_tables) { - if (with_table->next_global) + head->tables_pos.set_start_pos(&with_table->next_global); + head->tables_pos.set_end_pos(lex->query_tables_last); + TABLE_LIST *next_tbl= with_table->next_global; + if (next_tbl) { - spec_tables_tail->next_global= with_table->next_global; - with_table->next_global->prev_global= &spec_tables_tail->next_global; + *(lex->query_tables->prev_global= next_tbl->prev_global)= + lex->query_tables; + *(next_tbl->prev_global= lex->query_tables_last)= next_tbl; } else { - old_lex->query_tables_last= &spec_tables_tail->next_global; + *(lex->query_tables->prev_global= old_lex->query_tables_last)= + lex->query_tables; + old_lex->query_tables_last= lex->query_tables_last; } - spec_tables->prev_global= &with_table->next_global; - with_table->next_global= spec_tables; } res= &lex->unit; res->with_element= this; + /* + The unit of the specification that just has been parsed is included + as a slave of the select that contained in its from list the table + reference for which the unit has been created. + */ lex->unit.include_down(with_table->select_lex); - lex->unit.set_slave(with_select); + lex->unit.set_slave(with_select); + lex->unit.cloned_from= spec; old_lex->all_selects_list= (st_select_lex*) (lex->all_selects_list-> insert_chain_before( (st_select_lex_node **) &(old_lex->all_selects_list), with_select)); - if (check_dependencies_in_with_clauses(lex->with_clauses_list)) - res= NULL; + /* - Resolve references to CTE from the spec_tables list that has not - been resolved yet. + Now all references to the CTE defined outside of the cloned specification + has to be resolved. Additionally if old_lex->only_cte_resolution == false + for the table references that has not been resolved requests for mdl_locks + has to be set. */ - for (TABLE_LIST *tbl= spec_tables; - tbl; - tbl= tbl->next_global) + lex->only_cte_resolution= old_lex->only_cte_resolution; + if (lex->resolve_references_to_cte(lex->query_tables, + lex->query_tables_last)) { - if (!tbl->with) - tbl->with= with_select->find_table_def_in_with_clauses(tbl); - if (tbl == spec_tables_tail) - break; - } - if (check_table_access(thd, SELECT_ACL, spec_tables, FALSE, UINT_MAX, FALSE)) + res= NULL; goto err; + } - lex->sphead= NULL; // in order not to delete lex->sphead + lex->sphead= NULL; // in order not to delete lex->sphead lex_end(lex); err: - if (arena) - thd->restore_active_arena(arena, &backup); thd->lex= old_lex; return res; } @@ -1080,58 +1291,6 @@ With_element *st_select_lex::find_table_def_in_with_clauses(TABLE_LIST *table) } -/** - @brief - Set the specifying unit in this reference to a with table - - @details - The method assumes that the given element with_elem defines the table T - this table reference refers to. - If this is the first reference to T the method just sets its specification - in the field 'derived' as the unit that yields T. Otherwise the method - first creates a clone specification and sets rather this clone in this field. - - @retval - false on success - true on failure -*/ - -bool TABLE_LIST::set_as_with_table(THD *thd, With_element *with_elem) -{ - if (table) - { - /* - This table was prematurely identified as a temporary table. - We correct it here, but it's not a nice solution in the case - when the temporary table with this name is not used anywhere - else in the query. - */ - thd->mark_tmp_table_as_free_for_reuse(table); - table= 0; - } - with= with_elem; - schema_table= NULL; - if (!with_elem->is_referenced() || with_elem->is_recursive) - { - derived= with_elem->spec; - if (derived != select_lex->master_unit() && - !with_elem->is_recursive && - !is_with_table_recursive_reference()) - { - derived->move_as_slave(select_lex); - } - } - else - { - if(!(derived= with_elem->clone_parsed_spec(thd, this))) - return true; - } - derived->first_select()->linkage= DERIVED_TABLE_TYPE; - with_elem->inc_references(); - return false; -} - - bool TABLE_LIST::is_recursive_with_table() { return with && with->is_recursive; @@ -1231,7 +1390,7 @@ bool st_select_lex::check_unrestricted_recursive(bool only_standard_compliant) if (only_standard_compliant && with_elem->is_unrestricted()) { my_error(ER_NOT_STANDARD_COMPLIANT_RECURSIVE, - MYF(0), with_elem->query_name->str); + MYF(0), with_elem->get_name_str()); return true; } @@ -1372,7 +1531,7 @@ bool st_select_lex::check_subqueries_with_recursive_references() sl_master->derived && sl_master->derived->is_materialized_derived()) { my_error(ER_REF_TO_RECURSIVE_WITH_TABLE_IN_DERIVED, - MYF(0), with_elem->query_name->str); + MYF(0), with_elem->get_name_str()); return true; } if (!sl_master->item) @@ -1434,7 +1593,23 @@ void With_clause::print(String *str, enum_query_type query_type) void With_element::print(String *str, enum_query_type query_type) { - str->append(query_name); + str->append(get_name()); + if (column_list.elements) + { + List_iterator_fast<LEX_STRING> li(column_list); + str->append('('); + for (LEX_STRING *col_name= li++; ; ) + { + str->append(col_name); + col_name= li++; + if (!col_name) + { + str->append(')'); + break; + } + str->append(','); + } + } str->append(STRING_WITH_LEN(" as ")); str->append('('); spec->print(str, query_type); diff --git a/sql/sql_cte.h b/sql/sql_cte.h index 58f371d..5f30894 100644 --- a/sql/sql_cte.h +++ b/sql/sql_cte.h @@ -9,6 +9,39 @@ struct st_unit_ctxt_elem; /** + @class With_element_head + @brief Head of the definition of a CTE table + + It contains the name of the CTE and it contains the position of the subchain + of table references used in the definition in the global chain of table + references used in the query where this definition is encountered. +*/ + +class With_element_head : public Sql_alloc +{ + /* The name of the defined CTE */ + LEX_CSTRING *query_name; + +public: + /* + The structure describing the subchain of the table references used in + the specification of the defined CTE in the global chain of table + references used in the query. The structure is fully defined only + after the CTE definition has been parsed. + */ + TABLE_CHAIN tables_pos; + + With_element_head(LEX_CSTRING *name) + : query_name(name) + { + tables_pos.set_start_pos(0); + tables_pos.set_end_pos(0); + } + friend class With_element; +}; + + +/** @class With_element @brief Definition of a CTE table @@ -69,9 +102,22 @@ class With_element : public Sql_alloc subqueries and specifications of other with elements). */ uint references; + + /* + true <=> this With_element is referred in the query in which the + element is defined + */ + bool referenced; + + /* + true <=> this With_element is needed for the execution of the query + in which the element is defined + */ + bool is_used_in_query; + /* Unparsed specification of the query that specifies this element. - It used to build clones of the specification if they are needed. + It's used to build clones of the specification if they are needed. */ LEX_STRING unparsed_spec; /* Offset of the specification in the input string */ @@ -85,10 +131,11 @@ class With_element : public Sql_alloc public: /* - The name of the table introduced by this with elememt. The name - can be used in FROM lists of the queries in the scope of the element. + Contains the name of the defined With element and the position of + the subchain of the tables references used by its definition in the + global chain of TABLE_LIST objects created for the whole query. */ - LEX_STRING *query_name; + With_element_head *head; /* Optional list of column names to name the columns of the table introduced by this with element. It is used in the case when the names are not @@ -144,18 +191,27 @@ class With_element : public Sql_alloc /* List of Item_subselects containing recursive references to this CTE */ SQL_I_List<Item_subselect> sq_with_rec_ref; - With_element(LEX_STRING *name, + With_element(With_element_head *h, List <LEX_STRING> list, st_select_lex_unit *unit) : next(NULL), base_dep_map(0), derived_dep_map(0), sq_dep_map(0), work_dep_map(0), mutually_recursive(0), top_level_dep_map(0), sq_rec_ref(NULL), next_mutually_recursive(NULL), references(0), - query_name(name), column_list(list), spec(unit), + referenced(false), is_used_in_query(false), + head(h), column_list(list), spec(unit), is_recursive(false), rec_outer_references(0), with_anchor(false), level(0), rec_result(NULL) { unit->with_element= this; } + LEX_CSTRING *get_name() { return head->query_name; } + const char *get_name_str() { return get_name()->str; } + + void set_tables_start_pos(TABLE_LIST **pos) + { head->tables_pos.set_start_pos(pos); } + void set_tables_end_pos(TABLE_LIST **pos) + { head->tables_pos.set_end_pos(pos); } + bool check_dependencies_in_spec(); void check_dependencies_in_select(st_select_lex *sl, st_unit_ctxt_elem *ctxt, @@ -182,9 +238,9 @@ class With_element : public Sql_alloc bool set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, uint spec_offset); - st_select_lex_unit *clone_parsed_spec(THD *thd, TABLE_LIST *with_table); + st_select_lex_unit *clone_parsed_spec(LEX *old_lex, TABLE_LIST *with_table); - bool is_referenced() { return references != 0; } + bool is_referenced() { return referenced; } void inc_references() { references++; } @@ -242,6 +298,12 @@ class With_element : public Sql_alloc void prepare_for_next_iteration(); friend class With_clause; + + friend + bool LEX::resolve_references_to_cte(TABLE_LIST *tables, + TABLE_LIST **tables_last); + friend + bool LEX::resolve_references_to_cte_in_hanging_cte(); }; const uint max_number_of_elements_in_with_clause= sizeof(table_map)*8; @@ -338,8 +400,10 @@ class With_clause : public Sql_alloc friend class With_element; friend - bool - check_dependencies_in_with_clauses(With_clause *with_clauses_list); + bool LEX::check_dependencies_in_with_clauses(); + + friend + bool LEX::resolve_references_to_cte_in_hanging_cte(); }; inline diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index c534ba7..eaaa313 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -704,6 +704,8 @@ void lex_start(THD *thd) lex->subqueries= FALSE; lex->context_analysis_only= 0; lex->derived_tables= 0; + lex->with_cte_resolution= false; + lex->only_cte_resolution= false; lex->safe_to_cache_query= 1; lex->parsing_options.reset(); lex->empty_field_list_on_rset= 0; @@ -2089,6 +2091,7 @@ void st_select_lex_unit::init_query() is_view= false; with_clause= 0; with_element= 0; + cloned_from= 0; columns_are_renamed= false; } diff --git a/sql/sql_lex.h b/sql/sql_lex.h index fbc2bf7..03c06b9 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -666,6 +666,8 @@ class st_select_lex_unit: public st_select_lex_node { With_clause *with_clause; /* With element where this unit is used as the specification (if any) */ With_element *with_element; + /* The unit used as a CTE specification from which this unit is cloned */ + st_select_lex_unit *cloned_from; /* thread handler */ THD *thd; /* @@ -1169,7 +1171,9 @@ class st_select_lex: public st_select_lex_node } With_element *get_with_element() { - return master_unit()->with_element; + return master_unit()->cloned_from ? + master_unit()->cloned_from->with_element : + master_unit()->with_element; } With_element *find_table_def_in_with_clauses(TABLE_LIST *table); bool check_unrestricted_recursive(bool only_standard_compliant); @@ -2772,6 +2776,20 @@ struct LEX: public Query_tables_list uint16 create_view_algorithm; uint8 create_view_check; uint8 context_analysis_only; + /* + true <=> The parsed fragment requires resolution of references to CTE + at the end of parsing. This name resolution process involves searching + for possible dependencies between CTE defined in the parsed fragment and + detecting possible recursive references. + The flag is set to true if the fragment contains CTE definitions. + */ + bool with_cte_resolution; + /* + true <=> only resolution of references to CTE are required in the parsed + fragment, no checking of dependencies between CTE is required. + This flag is used only when parsing clones of CTE specifications. + */ + bool only_cte_resolution; bool local_file; bool check_exists; bool autocommit; @@ -3224,6 +3242,13 @@ struct LEX: public Query_tables_list { return !create_like() && !create_select(); } + + bool check_dependencies_in_with_clauses(); + bool resolve_references_to_cte_in_hanging_cte(); + bool check_cte_dependencies_and_resolve_references(); + bool resolve_references_to_cte(TABLE_LIST *tables, + TABLE_LIST **tables_last); + }; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 9436e11..f782c5c 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3220,9 +3220,6 @@ mysql_execute_command(THD *thd) thd->get_stmt_da()->opt_clear_warning_info(thd->query_id); } - if (check_dependencies_in_with_clauses(thd->lex->with_clauses_list)) - DBUG_RETURN(1); - #ifdef HAVE_REPLICATION if (unlikely(thd->slave_thread)) { @@ -7980,7 +7977,8 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ptr->db= table->db.str; ptr->db_length= table->db.length; } - else if (lex->copy_db_to(&ptr->db, &ptr->db_length)) + else if (!lex->with_cte_resolution && + lex->copy_db_to(&ptr->db, &ptr->db_length)) DBUG_RETURN(0); else ptr->is_fqtn= FALSE; @@ -7997,7 +7995,9 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ptr->table_name=table->table.str; ptr->table_name_length=table->table.length; - ptr->lock_type= lock_type; + ptr->lock_type= lock_type; + ptr->mdl_type= mdl_type; + ptr->table_options= table_options; ptr->updating= MY_TEST(table_options & TL_OPTION_UPDATING); /* TODO: remove TL_OPTION_FORCE_INDEX as it looks like it's not used */ ptr->force_index= MY_TEST(table_options & TL_OPTION_FORCE_INDEX); @@ -8676,8 +8676,10 @@ void st_select_lex::set_lock_for_tables(thr_lock_type lock_type, bool for_update { tables->lock_type= lock_type; tables->updating= for_update; - tables->mdl_request.set_type((lock_type >= TL_WRITE_ALLOW_WRITE) ? - MDL_SHARED_WRITE : MDL_SHARED_READ); + + if (tables->db && tables->db[0]) + tables->mdl_request.set_type((lock_type >= TL_WRITE_ALLOW_WRITE) ? + MDL_SHARED_WRITE : MDL_SHARED_READ); } DBUG_VOID_RETURN; } diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index a8a6710..922d7c9 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2413,9 +2413,6 @@ static bool check_prepared_statement(Prepared_statement *stmt) if (tables) thd->get_stmt_da()->opt_clear_warning_info(thd->query_id); - if (check_dependencies_in_with_clauses(thd->lex->with_clauses_list)) - goto error; - if (sql_command_flags[sql_command] & CF_HA_CLOSE) mysql_ha_rm_tables(thd, tables); diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 0701c52..675eeb8 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -432,12 +432,6 @@ bool mysql_create_view(THD *thd, TABLE_LIST *views, lex->link_first_table_back(view, link_to_local); view->open_type= OT_BASE_ONLY; - if (check_dependencies_in_with_clauses(lex->with_clauses_list)) - { - res= TRUE; - goto err; - } - WSREP_TO_ISOLATION_BEGIN(WSREP_MYSQL_DB, NULL, NULL); /* @@ -1399,9 +1393,6 @@ bool mysql_make_view(THD *thd, TABLE_SHARE *share, TABLE_LIST *table, TABLE_LIST *tbl; Security_context *security_ctx= 0; - if (check_dependencies_in_with_clauses(thd->lex->with_clauses_list)) - goto err; - /* Check rights to run commands (ANALYZE SELECT, EXPLAIN SELECT & SHOW CREATE) which show underlying tables. diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 387c77a..25826d2 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -981,6 +981,7 @@ Virtual_column_info *add_virtual_expression(THD *thd, Item *expr) class sp_label *splabel; class sp_name *spname; class sp_variable *spvar; + class With_element_head *with_element_head; class With_clause *with_clause; class Virtual_column_info *virtual_column; @@ -2067,7 +2068,7 @@ END_OF_INPUT %type <with_clause> opt_with_clause with_clause -%type <lex_str_ptr> query_name +%type <with_element_head> with_element_head %type <lex_str_list> opt_with_column_list @@ -2959,7 +2960,11 @@ call: lex->value_list.empty(); sp_add_used_routine(lex, thd, $2, TYPE_ENUM_PROCEDURE); } - opt_sp_cparam_list {} + opt_sp_cparam_list + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; /* CALL parameters */ @@ -3805,6 +3810,8 @@ sp_proc_stmt_return: { LEX *lex= Lex; sp_head *sp= lex->sphead; + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; if (sp->m_type != TYPE_ENUM_FUNCTION) my_yyabort_error((ER_SP_BADRETURN, MYF(0))); @@ -4866,12 +4873,16 @@ create_select_query_expression: { Select->set_braces(0); Select->set_with_clause($1); + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } union_clause | opt_with_clause SELECT_SYM create_select_part2 create_select_part3_union_not_ready create_select_part4 { Select->set_with_clause($1); + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } | '(' create_select_query_specification ')' | '(' create_select_query_specification ')' @@ -5578,6 +5589,8 @@ create_select_query_specification: create_select_part4 { Select->set_with_clause($1); + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } ; @@ -8454,6 +8467,8 @@ select: LEX *lex= Lex; lex->sql_command= SQLCOM_SELECT; lex->current_select->set_with_clause($1); + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } ; @@ -12161,6 +12176,8 @@ do: expr_list { Lex->insert_list= $3; + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } ; @@ -12377,7 +12394,10 @@ insert: Lex->current_select= &Lex->select_lex; } insert_field_spec opt_insert_update - {} + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; replace: @@ -12394,7 +12414,10 @@ replace: Lex->current_select= &Lex->select_lex; } insert_field_spec - {} + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; insert_lock_option: @@ -12586,7 +12609,11 @@ update: */ slex->set_lock_for_tables($3, slex->table_list.elements == 1); } - opt_where_clause opt_order_clause delete_limit_clause {} + opt_where_clause opt_order_clause delete_limit_clause + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; update_list: @@ -12675,6 +12702,8 @@ single_multi: { if (multi_delete_set_locks_and_link_aux_tables(Lex)) MYSQL_YYABORT; + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; } ; @@ -13641,7 +13670,10 @@ load: opt_xml_rows_identified_by opt_field_term opt_line_term opt_ignore_lines opt_field_or_var_spec opt_load_data_set_spec - {} + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; data_or_xml: @@ -14096,6 +14128,8 @@ with_clause: if (with_clause == NULL) MYSQL_YYABORT; Lex->derived_tables|= DERIVED_WITH; + Lex->with_cte_resolution= true; + Lex->with_cte_resolution= true; Lex->curr_with_clause= with_clause; with_clause->add_to_list(Lex->with_clauses_list_last_next); } @@ -14120,7 +14154,7 @@ with_list: with_list_element: - query_name + with_element_head opt_with_column_list { $2= new List<LEX_STRING> (Lex->with_column_list); @@ -14140,6 +14174,7 @@ with_list_element: if (elem->set_unparsed_spec(thd, spec_start, $8, (uint) (spec_start - query_start))) MYSQL_YYABORT; + elem->set_tables_end_pos(lex->query_tables_last); } ; @@ -14166,12 +14201,15 @@ with_column_list: ; -query_name: +with_element_head: ident { - $$= (LEX_STRING *) thd->memdup(&$1, sizeof(LEX_STRING)); - if ($$ == NULL) + LEX_CSTRING *name= + (LEX_CSTRING *) thd->memdup(&$1, sizeof(LEX_CSTRING)); + $$= new (thd->mem_root) With_element_head(name); + if (unlikely(name == NULL || $$ == NULL)) MYSQL_YYABORT; + $$->tables_pos.set_start_pos(Lex->query_tables_last); } ; @@ -15078,7 +15116,10 @@ set: sp_create_assignment_lex(thd, yychar == YYEMPTY); } start_option_value_list - {} + { + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } | SET STATEMENT_SYM { LEX *lex= Lex; @@ -16691,6 +16732,9 @@ view_select: &not_used); lex->parsing_options.allows_variable= TRUE; lex->current_select->set_with_clause($2); + if (Lex->check_cte_dependencies_and_resolve_references()) + MYSQL_YYABORT; + } ; diff --git a/sql/table.h b/sql/table.h index 83c72f7..69bd14b 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1860,6 +1860,29 @@ class Item_in_subselect; struct LEX; class Index_hint; + +/* + @struct TABLE_CHAIN + @brief Subchain of global chain of table references + + The structure contains a pointer to the address of the next_global + pointer to the first TABLE_LIST objectof the subchain and the address + of the next_global pointer to the element right after the last + TABLE_LIST object of the subchain. For an empty subchain both pointers + have the same value. +*/ + +struct TABLE_CHAIN +{ + TABLE_CHAIN() {} + + TABLE_LIST **start_pos; + TABLE_LIST ** end_pos; + + void set_start_pos(TABLE_LIST **pos) { start_pos= pos; } + void set_end_pos(TABLE_LIST **pos) { end_pos= pos; } +}; + struct TABLE_LIST { TABLE_LIST() {} /* Remove gcc warning */ @@ -2166,6 +2189,20 @@ struct TABLE_LIST /* call back function for asking handler about caching in query cache */ qc_engine_callback callback_func; thr_lock_type lock_type; + + /* + Two fields below are set during parsing this table reference in the cases + when the table reference can be potentially a reference to a CTE table. + In this cases the fact that the reference is a reference to a CTE or not + will be ascertained at the very end of parsing of the query when referencies + to CTE are resolved. For references to CTE and to derived tables no mdl + requests are needed while for other table references they are. If a request + is possibly postponed the info that allows to issue this request must be + saved in 'mdl_type' and 'table_options'. + */ + enum_mdl_type mdl_type; + ulong table_options; + uint outer_join; /* Which join type */ uint shared; /* Used in multi-upd */ size_t db_length;

1 0