- commits - lists.mariadb.org

[Commits] 6c2479afa92: MDEV-21314: Range Locking: individual rows are locked when scanning PK
by psergey 06 Jan '20

06 Jan '20

revision-id: 6c2479afa9280fc5ac6d7f904a9a8dca284c6f5b (fb-prod201903-274-g6c2479afa92) parent(s): 33a3184a56e03cec2a910ed3b7c67624e08ab993 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-01-06 18:55:43 +0300 message: MDEV-21314: Range Locking: individual rows are locked when scanning PK Remove redundant locking in several places: - RocksDB's Put and [Single]Delete should not acquire locks when Range Lock manager is used. - MyRocks should not re-read rows with GetForUpdate() if range locking is used. Exception to this is the slave thread: it doesn't do range locking when searching for rows to update. Because of that, it will keep the old behavior and call GetForUpdate. --- mysql-test/suite/rocksdb/r/range_locking.result | 5 +---- mysql-test/suite/rocksdb/r/range_locking_escalation.result | 2 +- mysql-test/suite/rocksdb/r/range_locking_rev_cf.result | 5 +---- mysql-test/suite/rocksdb/t/range_locking.inc | 2 +- rocksdb | 2 +- storage/rocksdb/ha_rocksdb.cc | 8 +++++--- 6 files changed, 10 insertions(+), 14 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/range_locking.result b/mysql-test/suite/rocksdb/r/range_locking.result index 3205045c3b3..3641fb1943e 100644 --- a/mysql-test/suite/rocksdb/r/range_locking.result +++ b/mysql-test/suite/rocksdb/r/range_locking.result @@ -108,13 +108,12 @@ $cf_id $trx_id 00${indexnr}80000019 - 01${indexnr}80000028 X rollback; begin; # The following will show a range lock on 2-9 and also a point lock on 10. -# This is how things currently work. +# This is how things currently work. (after MDEV-21314, not anymore) select * from t1 where pk between 2 and 9 for update; pk a # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode $cf_id $trx_id 00${indexnr}80000002 - 01${indexnr}80000009 X -$cf_id $trx_id 00${indexnr}8000000a X rollback; drop table t1; connection con1; @@ -437,7 +436,6 @@ pk1 pk2 a 3 0 30 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}8000000280000009 X $cf_id $trx_id 00${indexnr}80000003 - 01${indexnr}80000003 X rollback; # @@ -463,7 +461,6 @@ pk1 pk2 a 4 5 45 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}8000000480000004 X $cf_id $trx_id 00${indexnr}8000000480000005 - 01${indexnr}8000000480000008 X rollback; connection con1; diff --git a/mysql-test/suite/rocksdb/r/range_locking_escalation.result b/mysql-test/suite/rocksdb/r/range_locking_escalation.result index 7b94df0c092..ae4c01cad72 100644 --- a/mysql-test/suite/rocksdb/r/range_locking_escalation.result +++ b/mysql-test/suite/rocksdb/r/range_locking_escalation.result @@ -23,5 +23,5 @@ count(*) 10000 show status like 'rocksdb_locktree_escalation_count'; Variable_name Value -rocksdb_locktree_escalation_count 3328 +rocksdb_locktree_escalation_count 3321 drop table t0,t1; diff --git a/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result b/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result index c10ebf6bf87..d33d4872fab 100644 --- a/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result +++ b/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result @@ -108,13 +108,12 @@ $cf_id $trx_id 00${indexnr}80000028 - 01${indexnr}80000019 X rollback; begin; # The following will show a range lock on 2-9 and also a point lock on 10. -# This is how things currently work. +# This is how things currently work. (after MDEV-21314, not anymore) select * from t1 where pk between 2 and 9 for update; pk a # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode $cf_id $trx_id 00${indexnr}80000009 - 01${indexnr}80000002 X -$cf_id $trx_id 00${indexnr}8000000a X rollback; drop table t1; connection con1; @@ -397,7 +396,6 @@ pk1 pk2 a 3 0 30 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}8000000280000009 X $cf_id $trx_id 00${indexnr}80000003 - 01${indexnr}80000003 X rollback; # @@ -423,7 +421,6 @@ pk1 pk2 a 4 5 45 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}8000000480000004 X $cf_id $trx_id 00${indexnr}8000000480000008 - 01${indexnr}8000000480000005 X rollback; connection con1; diff --git a/mysql-test/suite/rocksdb/t/range_locking.inc b/mysql-test/suite/rocksdb/t/range_locking.inc index fd0f3f4e6c6..c676ccb66db 100644 --- a/mysql-test/suite/rocksdb/t/range_locking.inc +++ b/mysql-test/suite/rocksdb/t/range_locking.inc @@ -132,7 +132,7 @@ delete from t1 where pk between 25 and 40; rollback; begin; --echo # The following will show a range lock on 2-9 and also a point lock on 10. ---echo # This is how things currently work. +--echo # This is how things currently work. (after MDEV-21314, not anymore) select * from t1 where pk between 2 and 9 for update; --source suite/rocksdb/include/select_from_is_rowlocks.inc rollback; diff --git a/rocksdb b/rocksdb index 1c9d747b0e4..6b3b684f69a 160000 --- a/rocksdb +++ b/rocksdb @@ -1 +1 @@ -Subproject commit 1c9d747b0e426df3dff4bc70f1ff7ba0271621b0 +Subproject commit 6b3b684f69a5b6ad968f2270e54e92de1a42d468 diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index b00801c62ab..44ab3ef36df 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -8187,7 +8187,7 @@ int ha_rocksdb::read_row_from_primary_key(uchar *const buf) { memcpy(m_pk_packed_tuple, pk_data, pk_size); m_last_rowkey.copy(pk_data, pk_size, &my_charset_bin); - if (m_lock_rows != RDB_LOCK_NONE) { + if (m_lock_rows != RDB_LOCK_NONE && !rocksdb_use_range_locking) { /* We need to put a lock and re-read */ rc = get_row_by_rowid(buf, m_pk_packed_tuple, pk_size); } else { @@ -10796,7 +10796,8 @@ void ha_rocksdb::setup_iterator_for_rnd_scan() { 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_lock_rows != RDB_LOCK_NONE) && rocksdb_use_range_locking); + (m_lock_rows != RDB_LOCK_NONE) && + rocksdb_use_range_locking && !ha_thd()->rli_slave); m_scan_it->Seek(table_key); m_skip_scan_it_next_call = true; } @@ -10918,7 +10919,8 @@ int ha_rocksdb::rnd_next_with_direction(uchar *const buf, bool move_forward) { break; } - if (m_lock_rows != RDB_LOCK_NONE) { + if (m_lock_rows != RDB_LOCK_NONE && (!rocksdb_use_range_locking || + ha_thd()->rli_slave)) { /* Lock the row we've just read.

1 0

[Commits] 6b3b684f6: Make Transaction_BaseImpl::{Put, Delete, SingleDelete} not take locks in range-locking mode
by psergey 05 Jan '20

05 Jan '20

revision-id: 6b3b684f69a5b6ad968f2270e54e92de1a42d468 (v5.8-1903-g6b3b684f6) parent(s): a35d075533e60892819eeaedd63744bf538e4386 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-01-05 22:22:13 +0300 message: Make Transaction_BaseImpl::{Put,Delete,SingleDelete} not take locks in range-locking mode --- utilities/transactions/transaction_base.cc | 42 +++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/utilities/transactions/transaction_base.cc b/utilities/transactions/transaction_base.cc index bbbfc8bc6..f9a8b3b74 100644 --- a/utilities/transactions/transaction_base.cc +++ b/utilities/transactions/transaction_base.cc @@ -379,8 +379,11 @@ Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family, const Slice& key, const Slice& value, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->Put(column_family, key, value); @@ -396,8 +399,11 @@ Status TransactionBaseImpl::Put(ColumnFamilyHandle* column_family, const SliceParts& key, const SliceParts& value, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->Put(column_family, key, value); @@ -430,8 +436,11 @@ Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family, const Slice& key, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->Delete(column_family, key); @@ -447,8 +456,11 @@ Status TransactionBaseImpl::Delete(ColumnFamilyHandle* column_family, const SliceParts& key, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->Delete(column_family, key); @@ -464,8 +476,11 @@ Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family, const Slice& key, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->SingleDelete(column_family, key); @@ -481,8 +496,11 @@ Status TransactionBaseImpl::SingleDelete(ColumnFamilyHandle* column_family, const SliceParts& key, const bool assume_tracked) { const bool do_validate = !assume_tracked; - Status s = TryLock(column_family, key, false /* read_only */, - true /* exclusive */, do_validate, assume_tracked); + Status s; + if (do_key_tracking_) { + s = TryLock(column_family, key, false /* read_only */, + true /* exclusive */, do_validate, assume_tracked); + } if (s.ok()) { s = GetBatchForWrite()->SingleDelete(column_family, key);

1 0

[Commits] a35d07553: Range Locking: fix compile warning/failure, "unused parameter"
by psergey 05 Jan '20

05 Jan '20

revision-id: a35d075533e60892819eeaedd63744bf538e4386 (v5.8-1902-ga35d07553) parent(s): 1c9d747b0e426df3dff4bc70f1ff7ba0271621b0 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-01-05 22:20:51 +0300 message: Range Locking: fix compile warning/failure, "unused parameter" --- utilities/transactions/range_locking/locktree/locktree.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities/transactions/range_locking/locktree/locktree.cc b/utilities/transactions/range_locking/locktree/locktree.cc index 4c7ce1bfe..3c32e1b77 100644 --- a/utilities/transactions/range_locking/locktree/locktree.cc +++ b/utilities/transactions/range_locking/locktree/locktree.cc @@ -356,7 +356,7 @@ static bool iterate_and_get_overlapping_row_locks2(const concurrent_tree::locked_keyrange *lkr, const DBT *left_key, const DBT *right_key, comparator *cmp, - TXNID txnid, + TXNID, GrowableArray<row_lock> *row_locks) { struct copy_fn_obj { GrowableArray<row_lock> *row_locks;

1 0

[Commits] 33a3184a56e: Range Locking: previous cset cont'd: fix assertions with reverse-order CFs
by Sergei Petrunia 05 Jan '20

05 Jan '20

revision-id: 33a3184a56e03cec2a910ed3b7c67624e08ab993 (fb-prod201903-273-g33a3184a56e) parent(s): f9cc0d345aeccabcb84ac98ce5d1130b637e024d author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-01-05 18:04:51 +0300 message: Range Locking: previous cset cont'd: fix assertions with reverse-order CFs - Endpoint flags must be flipped whenever we are using reverse-ordered CFs. - As for range endpoints themselves = they must be flipped when doing a reverse-ordered scan = they must NOT be flipped when using reverse-ordered CF. (but the code in index_first/index_last already "flips" them by calling index_(first|last)_intern. Take all of the above into account. --- mysql-test/suite/rocksdb/r/range_locking.result | 2 +- .../suite/rocksdb/r/range_locking_rev_cf.result | 2 +- storage/rocksdb/ha_rocksdb.cc | 23 +++++++++++++++------- storage/rocksdb/ha_rocksdb.h | 1 + 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/mysql-test/suite/rocksdb/r/range_locking.result b/mysql-test/suite/rocksdb/r/range_locking.result index 85264fbf2fa..3205045c3b3 100644 --- a/mysql-test/suite/rocksdb/r/range_locking.result +++ b/mysql-test/suite/rocksdb/r/range_locking.result @@ -510,7 +510,7 @@ pk a 1992 1992 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}800007c6 - 01${indexnr+1} X +$cf_id $trx_id 00${indexnr}800007c6 - 00${indexnr+1} X rollback; connection con1; rollback; diff --git a/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result b/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result index 87522d5ae37..c10ebf6bf87 100644 --- a/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result +++ b/mysql-test/suite/rocksdb/r/range_locking_rev_cf.result @@ -470,7 +470,7 @@ pk a 1992 1992 # select * from information_schema.rocksdb_locks; # With replacements by select_from_is_rowlocks.inc COLUMN_FAMILY_ID TRANSACTION_ID KEY mode -$cf_id $trx_id 00${indexnr}800007c6 - 01${indexnr+1} X +$cf_id $trx_id 00${indexnr}800007c6 - 00${indexnr+1} X rollback; connection con1; rollback; diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index f77da1bc8da..b00801c62ab 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -8393,6 +8393,7 @@ int ha_rocksdb::read_range_first(const key_range *const start_key, if (!start_key) { // Read first record + // (range locking will set the lock inside the ha_index_first call) result = ha_index_first(table->record[0]); } else { if (is_using_prohibited_gap_locks( @@ -8443,6 +8444,7 @@ int ha_rocksdb::set_range_lock(Rdb_transaction *tx, const enum ha_rkey_function &find_flag, const rocksdb::Slice &slice_arg, const key_range *const end_key, + bool flip_rev_cf, bool *use_locking_iterator ) { @@ -8591,12 +8593,19 @@ int ha_rocksdb::set_range_lock(Rdb_transaction *tx, no_end_endpoint= true; } + if (kd.m_is_reverse_cf) { + // Flip the endpoint flags + end_has_inf_suffix = !end_has_inf_suffix; + start_has_inf_suffix = !start_has_inf_suffix; + } + rocksdb::Endpoint start_endp; rocksdb::Endpoint end_endp; - if (kd.m_is_reverse_cf) { + + if (flip_rev_cf && kd.m_is_reverse_cf) { // Flip the endpoints - start_endp =rocksdb::Endpoint(end_slice, !end_has_inf_suffix); - end_endp = rocksdb::Endpoint(slice, !start_has_inf_suffix); + start_endp =rocksdb::Endpoint(end_slice, end_has_inf_suffix); + end_endp = rocksdb::Endpoint(slice, start_has_inf_suffix); } else { start_endp= rocksdb::Endpoint(slice, start_has_inf_suffix); end_endp= rocksdb::Endpoint(end_slice, end_has_inf_suffix); @@ -8747,7 +8756,7 @@ int ha_rocksdb::index_read_map_impl(uchar *const buf, const uchar *const key, bool use_locking_iterator; rocksdb::Slice lock_slice(reinterpret_cast<const char *>(m_sk_packed_tuple), packed_size); - if ((rc = set_range_lock(tx, kd, find_flag, lock_slice, end_key, + if ((rc = set_range_lock(tx, kd, find_flag, lock_slice, end_key, true, &use_locking_iterator))) DBUG_RETURN(rc); @@ -9432,7 +9441,7 @@ int ha_rocksdb::index_first_intern(uchar *const buf) { bool use_locking_iter; if ((rc = set_range_lock(tx, kd, HA_READ_KEY_OR_NEXT, index_key, - end_range, &use_locking_iter))) + end_range, false, &use_locking_iter))) DBUG_RETURN(rc); const bool is_new_snapshot = !tx->has_snapshot(); @@ -9527,8 +9536,8 @@ int ha_rocksdb::index_last_intern(uchar *const buf) { DBUG_ASSERT(tx != nullptr); bool use_locking_iter; - if ((rc = set_range_lock(tx, kd, HA_READ_PREFIX_LAST_OR_PREV, index_key, - end_range, &use_locking_iter))) + if ((rc = set_range_lock(tx, kd, HA_READ_BEFORE_KEY, index_key, + end_range, false, &use_locking_iter))) DBUG_RETURN(rc); bool is_new_snapshot = !tx->has_snapshot(); diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index 272dc9973d4..cbe894df8f5 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/storage/rocksdb/ha_rocksdb.h @@ -329,6 +329,7 @@ class ha_rocksdb : public my_core::handler { const enum ha_rkey_function &find_flag, const rocksdb::Slice &slice, const key_range *const end_key, + bool flip_rev_cf, bool *use_locking_iterator); void release_scan_iterator(void);

1 0

[Commits] d13f3a4: MDEV-21184 Assertion `used_tables_cache == 0' failed in Item_func::fix_fields
by IgorBabaev 03 Jan '20

03 Jan '20

revision-id: d13f3a43c0fffd0daffbaf564760b73d6228b171 (mariadb-10.4.10-33-gd13f3a4) parent(s): ed355f59dd7e0065ebde15223c2f39f8b71b2958 author: Igor Babaev committer: Igor Babaev timestamp: 2020-01-03 11:15:00 -0800 message: MDEV-21184 Assertion `used_tables_cache == 0' failed in Item_func::fix_fields with condition_pushdown_from_having This bug could manifest itself for queries with GROUP BY and HAVING clauses when the HAVING clause was a conjunctive condition that depended exclusively on grouping fields and at least one conjunct contained an equality of the form fld=sq where fld is a grouping field and sq is a constant subquery. In this case the optimizer tries to perform a pushdown of the HAVING condition into WHERE. To construct the pushable condition the optimizer first transforms all multiple equalities in HAVING into simple equalities. This has to be done for a proper processing of the pushed conditions in WHERE. The multiple equalities at all AND/OR levels must be converted to simple equalities because any multiple equality may refer to a multiple equality at the upper level. Before this patch the conversion was performed like this: multiple_equality(x,f1,...,fn) => x=f1 and ... and x=fn. When an equality item for x=fi was constructed both the items for x and fi were cloned. If x happened to be a constant subquery that could not be cloned the conversion failed. If the conversions of multiple equalities previously performed had succeeded then the whole condition became in an inconsistent state that could cause different failures. The solution provided by the patch is: 1. to use a different conversion rule if x is a constant multiple_equality(x,f1,...,fn) => f1=x and f2=f1 and ... and fn=f1 2. not to clone x if it's a constant. Such conversions cannot fail and besides the result of the conversion preserves the equivalence of f1,...,fn that can be used for other optimizations. This patch also made sure that expensive predicates are not pushed from HAVING to WHERE. --- mysql-test/main/derived_cond_pushdown.result | 8 +- mysql-test/main/having_cond_pushdown.result | 148 +++++++++++++++++++++++++++ mysql-test/main/having_cond_pushdown.test | 39 +++++++ sql/item.cc | 14 ++- sql/item.h | 7 +- sql/item_cmpfunc.cc | 105 +++++++++++-------- sql/item_cmpfunc.h | 3 +- sql/sql_lex.cc | 14 ++- 8 files changed, 280 insertions(+), 58 deletions(-) diff --git a/mysql-test/main/derived_cond_pushdown.result b/mysql-test/main/derived_cond_pushdown.result index c044b79..125de26 100644 --- a/mysql-test/main/derived_cond_pushdown.result +++ b/mysql-test/main/derived_cond_pushdown.result @@ -8937,13 +8937,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1" + "attached_condition": "t1.a = 1 and t1.b = 1" } } } @@ -9012,13 +9012,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1 and t1.d = 1" + "attached_condition": "t1.a = 1 and t1.b = 1 and t1.d = 1" } } } diff --git a/mysql-test/main/having_cond_pushdown.result b/mysql-test/main/having_cond_pushdown.result index 82a4813..9b12429 100644 --- a/mysql-test/main/having_cond_pushdown.result +++ b/mysql-test/main/having_cond_pushdown.result @@ -4776,3 +4776,151 @@ WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE DROP TABLE t1; +# +# MDEV-21184: Constant subquery in condition movable to WHERE +# +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES +(1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a, t1.b", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "(t1.a = 8 or t1.a = (subquery#2)) and t1.b < 20" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +a +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "having_condition": "sum(t1.b) > 20", + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } +} +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +a +2 +DROP TABLE t1,t2; diff --git a/mysql-test/main/having_cond_pushdown.test b/mysql-test/main/having_cond_pushdown.test index f1bf706..fc75122 100644 --- a/mysql-test/main/having_cond_pushdown.test +++ b/mysql-test/main/having_cond_pushdown.test @@ -1401,3 +1401,42 @@ EXPLAIN SELECT t1.a,MAX(t1.b),t1.c FROM t1 WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; DROP TABLE t1; + +--echo # +--echo # MDEV-21184: Constant subquery in condition movable to WHERE +--echo # + +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES + (1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); + +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a,b + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +DROP TABLE t1,t2; diff --git a/sql/item.cc b/sql/item.cc index 900a973..7b4571e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -7352,7 +7352,7 @@ Item *Item::build_pushable_cond(THD *thd, List<Item> equalities; Item *new_cond= NULL; if (((Item_equal *)this)->create_pushable_equalities(thd, &equalities, - checker, arg) || + checker, arg, true) || (equalities.elements == 0)) return 0; @@ -10512,3 +10512,15 @@ void Item::register_in(THD *thd) next= thd->free_list; thd->free_list= this; } + + +bool Item::cleanup_excluding_immutables_processor (void *arg) +{ + if (!(get_extraction_flag() == IMMUTABLE_FL)) + return cleanup_processor(arg); + else + { + clear_extraction_flag(); + return false; + } +} diff --git a/sql/item.h b/sql/item.h index 2ac0964..205c070 100644 --- a/sql/item.h +++ b/sql/item.h @@ -152,8 +152,10 @@ bool mark_unsupported_function(const char *w1, const char *w2, #define NO_EXTRACTION_FL (1 << 6) #define FULL_EXTRACTION_FL (1 << 7) #define DELETION_FL (1 << 8) -#define SUBSTITUTION_FL (1 << 9) -#define EXTRACTION_MASK (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL) +#define IMMUTABLE_FL (1 << 9) +#define SUBSTITUTION_FL (1 << 10) +#define EXTRACTION_MASK \ + (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL | IMMUTABLE_FL) extern const char *item_empty_name; @@ -1867,6 +1869,7 @@ class Item: public Value_source, virtual bool cleanup_processor(void *arg); virtual bool cleanup_excluding_fields_processor (void *arg) { return cleanup_processor(arg); } + bool cleanup_excluding_immutables_processor (void *arg); virtual bool cleanup_excluding_const_fields_processor (void *arg) { return cleanup_processor(arg); } virtual bool collect_item_field_processor(void *arg) { return 0; } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 9110f34..5ae5931 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -7410,6 +7410,7 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) of the tree of the object to check if multiple equality elements can be used to create equalities @param arg parameter to be passed to the checker + @param clone_const true <=> clone the constant member if there is any @details How the method works on examples: @@ -7420,36 +7421,31 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) Example 2: It takes MULT_EQ(1,a,b) and tries to create from its elements a set of - equalities {(1=a),(1=b)}. + equalities {(a=1),(a=b)}. How it is done: - 1. The method finds the left part of the equalities to be built. It will - be the same for all equalities. It is either: - a. A constant if there is any - b. A first element in the multiple equality that satisfies - checker function + 1. If there is a constant member c the first non-constant member x for + which the function checker returns true is taken and an item for + the equality x=c is created. When constructing the equality item + the left part of the equality is always taken as a clone of x while + the right part is taken as a clone of c only if clone_const == true. - For the example 1 the left element is field 'x'. - For the example 2 it is constant '1'. + 2. After this all equalities of the form x=a (where x designates the first + non-constant member for which checker returns true and a is some other + such member of the multiplle equality) are created. When constructing + an equality item both its parts are taken as clones of x and a. - 2. If the left element is found the rest elements of the multiple equality - are checked with the checker function if they can be right parts - of equalities. - If the element can be a right part of the equality, equality is built. - It is built with the left part element found at the step 1 and - the right part element found at this step (step 2). - - Suppose for the example above that both 'a' and 'b' fields can be used - to build equalities: + Suppose in the examples above that for 'x', 'a', and 'b' the function + checker returns true. Example 1: - for 'a' field (x=a) is built - for 'b' field (x=b) is built + the equality (x=a) is built + the equality (x=b) is built Example 2: - for 'a' field (1=a) is built - for 'b' field (1=b) is built + the equality (a=1) is built + the equality (a=b) is built 3. As a result we get a set of equalities built with the elements of this multiple equality. They are saved in the equality list. @@ -7458,15 +7454,17 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) {(x=a),(x=b)} Example 2: - {(1=a),(1=b)} + {(a=1),(a=b)} @note This method is called for condition pushdown into materialized derived table/view, and IN subquery, and pushdown from HAVING into WHERE. When it is called for pushdown from HAVING the empty checker is passed. - It happens because elements of this multiple equality don't need to be - checked if they can be used to build equalities. There are no elements - that can't be used to build equalities. + This is because in this case the elements of the multiple equality don't + need to be checked if they can be used to build equalities: either all + equalities can be pushed or none of them can be pushed. + When the function is called for pushdown from HAVING the value of the + parameter clone_const is always false. In other cases it's always true. @retval true if an error occurs @retval false otherwise @@ -7475,24 +7473,42 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) bool Item_equal::create_pushable_equalities(THD *thd, List<Item> *equalities, Pushdown_checker checker, - uchar *arg) + uchar *arg, + bool clone_const) { Item *item; + Item *left_item= NULL; + Item *right_item = get_const(); Item_equal_fields_iterator it(*this); - Item *left_item = get_const(); - if (!left_item) + + while ((item=it++)) { - while ((item=it++)) - { - left_item= item; - if (checker && !((item->*checker) (arg))) - continue; - break; - } + left_item= item; + if (checker && !((item->*checker) (arg))) + continue; + break; } + if (!left_item) return false; + if (right_item) + { + Item_func_eq *eq= 0; + Item *left_item_clone= left_item->build_clone(thd); + Item *right_item_clone= !clone_const ? + right_item : right_item->build_clone(thd); + if (!left_item_clone || !right_item_clone) + return true; + eq= new (thd->mem_root) Item_func_eq(thd, + left_item_clone, + right_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) + return true; + if (!clone_const) + right_item->set_extraction_flag(IMMUTABLE_FL); + } + while ((item=it++)) { if (checker && !((item->*checker) (arg))) @@ -7500,15 +7516,14 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item_func_eq *eq= 0; Item *left_item_clone= left_item->build_clone(thd); Item *right_item_clone= item->build_clone(thd); - if (left_item_clone && right_item_clone) - { - left_item_clone->set_item_equal(NULL); - right_item_clone->set_item_equal(NULL); - eq= new (thd->mem_root) Item_func_eq(thd, - right_item_clone, - left_item_clone); - } - if (eq && equalities->push_back(eq, thd->mem_root)) + if (!(left_item_clone && right_item_clone)) + return true; + left_item_clone->set_item_equal(NULL); + right_item_clone->set_item_equal(NULL); + eq= new (thd->mem_root) Item_func_eq(thd, + right_item_clone, + left_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) return true; } return false; @@ -7533,7 +7548,7 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item *Item_equal::multiple_equality_transformer(THD *thd, uchar *arg) { List<Item> equalities; - if (create_pushable_equalities(thd, &equalities, 0, 0)) + if (create_pushable_equalities(thd, &equalities, 0, 0, false)) return 0; switch (equalities.elements) diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 0a91d45..1d84ee6 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3208,7 +3208,8 @@ class Item_equal: public Item_bool_func bool excl_dep_on_in_subq_left_part(Item_in_subselect *subq_pred); bool excl_dep_on_grouping_fields(st_select_lex *sel); bool create_pushable_equalities(THD *thd, List<Item> *equalities, - Pushdown_checker checker, uchar *arg); + Pushdown_checker checker, uchar *arg, + bool clone_const); /* Return the number of elements in this multiple equality */ uint elements_count() { return equal_items.elements; } friend class Item_equal_fields_iterator; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 16bb53c..ea34679 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -7988,7 +7988,7 @@ st_select_lex::check_cond_extraction_for_grouping_fields(THD *thd, Item *cond) } else { - int fl= cond->excl_dep_on_grouping_fields(this) ? + int fl= cond->excl_dep_on_grouping_fields(this) && !cond->is_expensive() ? FULL_EXTRACTION_FL : NO_EXTRACTION_FL; cond->set_extraction_flag(fl); } @@ -9819,7 +9819,7 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) { List_iterator<Item> li(*((Item_cond*) result)->argument_list()); Item *item; - while ((item=li++)) + while ((item= li++)) { if (attach_to_conds.push_back(item, thd->mem_root)) return true; @@ -9839,8 +9839,13 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) */ if (cond->type() != Item::COND_ITEM) return false; + if (((Item_cond *)cond)->functype() != Item_cond::COND_AND_FUNC) { + /* + cond is not a conjunctive formula and it cannot be pushed into WHERE. + Try to extract a formula that can be pushed. + */ Item *fix= cond->build_pushable_cond(thd, 0, 0); if (!fix) return false; @@ -9860,7 +9865,6 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) Item *result= item->transform(thd, &Item::multiple_equality_transformer, (uchar *)item); - if (!result) return true; if (result->type() == Item::COND_ITEM && @@ -10188,8 +10192,8 @@ Item *st_select_lex::pushdown_from_having_into_where(THD *thd, Item *having) &Item::field_transformer_for_having_pushdown, (uchar *)this); - if (item->walk(&Item:: cleanup_processor, 0, STOP_PTR) || - item->fix_fields(thd, NULL)) + if (item->walk(&Item::cleanup_excluding_immutables_processor, 0, STOP_PTR) + || item->fix_fields(thd, NULL)) { attach_to_conds.empty(); goto exit;

1 0

[Commits] 547495b: MDEV-21184 Assertion `used_tables_cache == 0' failed in Item_func::fix_fields
by IgorBabaev 03 Jan '20

03 Jan '20

revision-id: 547495bacdeb7de5bfa135159879d3c3ac894d1d (mariadb-10.4.10-33-g547495b) parent(s): ed355f59dd7e0065ebde15223c2f39f8b71b2958 author: Igor Babaev committer: Igor Babaev timestamp: 2020-01-03 11:12:51 -0800 message: MDEV-21184 Assertion `used_tables_cache == 0' failed in Item_func::fix_fields with condition_pushdown_from_having This bug could manifest itself for queries with GROUP BY and HAVING clauses when the HAVING clause was a conjunctive condition that depended exclusively on grouping fields and at least one conjunct contained an equality of the form fld=sq where fld is a grouping field and sq is a constant subquery. In this case the optimizer tries to perform a pushdown of the HAVING condition into WHERE. To construct the pushable condition the optimizer first transforms all multiple equalities in HAVING into simple equalities. This has to be done for a proper processing of the pushed conditions in WHERE. The multiple equalities at all AND/OR levels must be converted to simple equalities because any multiple equality may refer to a multiple equality at the upper level. Before this patch the conversion was performed like this: multiple_equality(x,f1,...,fn) => x=f1 and ... and x=fn. When an equality item for x=fi was constructed both the items for x and fi were cloned. If x happened to be a constant subquery that could not be cloned the conversion failed. If the conversions of multiple equalities previously performed had succeeded then the whole condition became in an inconsistent state that could cause different failures. The solution provided by the patch is: 1. to use a different conversion rule if x is a constant multiple_equality(x,f1,...,fn) => f1=x and f2=f1 and ... and fn=f1 2. not to clone x if it's a constant. Such conversions cannot fail and besides the result of the conversion preserves the equivalence of f1,...,fn that can be used for other optimizations. This patch also made sure that expensive predicates are not pushed from HAVING to WHERE. --- mysql-test/main/derived_cond_pushdown.result | 8 +- mysql-test/main/having_cond_pushdown.result | 148 +++++++++++++++++++++++++++ mysql-test/main/having_cond_pushdown.test | 39 +++++++ sql/item.cc | 14 ++- sql/item.h | 7 +- sql/item_cmpfunc.cc | 105 +++++++++++-------- sql/sql_lex.cc | 14 ++- 7 files changed, 278 insertions(+), 57 deletions(-) diff --git a/mysql-test/main/derived_cond_pushdown.result b/mysql-test/main/derived_cond_pushdown.result index c044b79..125de26 100644 --- a/mysql-test/main/derived_cond_pushdown.result +++ b/mysql-test/main/derived_cond_pushdown.result @@ -8937,13 +8937,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1" + "attached_condition": "t1.a = 1 and t1.b = 1" } } } @@ -9012,13 +9012,13 @@ EXPLAIN "materialized": { "query_block": { "select_id": 2, - "having_condition": "t1.b = 1 and max_c > 37 and max_c > 30", + "having_condition": "max_c > 37 and max_c > 30", "table": { "table_name": "t1", "access_type": "ALL", "rows": 3, "filtered": 100, - "attached_condition": "t1.a = 1 and t1.d = 1" + "attached_condition": "t1.a = 1 and t1.b = 1 and t1.d = 1" } } } diff --git a/mysql-test/main/having_cond_pushdown.result b/mysql-test/main/having_cond_pushdown.result index 82a4813..9b12429 100644 --- a/mysql-test/main/having_cond_pushdown.result +++ b/mysql-test/main/having_cond_pushdown.result @@ -4776,3 +4776,151 @@ WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE DROP TABLE t1; +# +# MDEV-21184: Constant subquery in condition movable to WHERE +# +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES +(1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "filesort": { + "sort_key": "t1.a, t1.b", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "(t1.a = 8 or t1.a = (subquery#2)) and t1.b < 20" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a,b +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; +a +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +EXPLAIN +{ + "query_block": { + "select_id": 1, + "having_condition": "sum(t1.b) > 20", + "filesort": { + "sort_key": "t1.a", + "temporary_table": { + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = 8 or t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } + } + } +} +SELECT a FROM t1 GROUP BY a +HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; +a +2 +EXPLAIN FORMAT=JSON SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +EXPLAIN +{ + "query_block": { + "select_id": 1, + "table": { + "table_name": "t1", + "access_type": "ALL", + "rows": 7, + "filtered": 100, + "attached_condition": "t1.a = (subquery#2)" + }, + "subqueries": [ + { + "query_block": { + "select_id": 2, + "table": { + "table_name": "t2", + "access_type": "ALL", + "rows": 2, + "filtered": 100 + } + } + } + ] + } +} +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); +a +2 +DROP TABLE t1,t2; diff --git a/mysql-test/main/having_cond_pushdown.test b/mysql-test/main/having_cond_pushdown.test index f1bf706..fc75122 100644 --- a/mysql-test/main/having_cond_pushdown.test +++ b/mysql-test/main/having_cond_pushdown.test @@ -1401,3 +1401,42 @@ EXPLAIN SELECT t1.a,MAX(t1.b),t1.c FROM t1 WHERE t1.a = 3 AND (t1.a < 2 AND t1.b > 3) GROUP BY t1.a; DROP TABLE t1; + +--echo # +--echo # MDEV-21184: Constant subquery in condition movable to WHERE +--echo # + +CREATE TABLE t1(a int, b int); +INSERT INTO t1 VALUES + (1,10), (2,20), (1,11), (1,15), (2,20), (1,10), (2,21); + +CREATE TABLE t2 (c INT); +INSERT INTO t2 VALUES (2),(3); + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = 8 OR a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a,b + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and b < 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a + HAVING ( a = 8 OR a = ( SELECT MIN(c) FROM t2 ) ) and SUM(b) > 20; + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +let $q= +SELECT a FROM t1 GROUP BY a HAVING a = ( SELECT MIN(c) FROM t2 ); + +eval EXPLAIN FORMAT=JSON $q; +eval $q; + +DROP TABLE t1,t2; diff --git a/sql/item.cc b/sql/item.cc index 900a973..7b4571e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -7352,7 +7352,7 @@ Item *Item::build_pushable_cond(THD *thd, List<Item> equalities; Item *new_cond= NULL; if (((Item_equal *)this)->create_pushable_equalities(thd, &equalities, - checker, arg) || + checker, arg, true) || (equalities.elements == 0)) return 0; @@ -10512,3 +10512,15 @@ void Item::register_in(THD *thd) next= thd->free_list; thd->free_list= this; } + + +bool Item::cleanup_excluding_immutables_processor (void *arg) +{ + if (!(get_extraction_flag() == IMMUTABLE_FL)) + return cleanup_processor(arg); + else + { + clear_extraction_flag(); + return false; + } +} diff --git a/sql/item.h b/sql/item.h index 2ac0964..205c070 100644 --- a/sql/item.h +++ b/sql/item.h @@ -152,8 +152,10 @@ bool mark_unsupported_function(const char *w1, const char *w2, #define NO_EXTRACTION_FL (1 << 6) #define FULL_EXTRACTION_FL (1 << 7) #define DELETION_FL (1 << 8) -#define SUBSTITUTION_FL (1 << 9) -#define EXTRACTION_MASK (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL) +#define IMMUTABLE_FL (1 << 9) +#define SUBSTITUTION_FL (1 << 10) +#define EXTRACTION_MASK \ + (NO_EXTRACTION_FL | FULL_EXTRACTION_FL | DELETION_FL | IMMUTABLE_FL) extern const char *item_empty_name; @@ -1867,6 +1869,7 @@ class Item: public Value_source, virtual bool cleanup_processor(void *arg); virtual bool cleanup_excluding_fields_processor (void *arg) { return cleanup_processor(arg); } + bool cleanup_excluding_immutables_processor (void *arg); virtual bool cleanup_excluding_const_fields_processor (void *arg) { return cleanup_processor(arg); } virtual bool collect_item_field_processor(void *arg) { return 0; } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 9110f34..5ae5931 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -7410,6 +7410,7 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) of the tree of the object to check if multiple equality elements can be used to create equalities @param arg parameter to be passed to the checker + @param clone_const true <=> clone the constant member if there is any @details How the method works on examples: @@ -7420,36 +7421,31 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) Example 2: It takes MULT_EQ(1,a,b) and tries to create from its elements a set of - equalities {(1=a),(1=b)}. + equalities {(a=1),(a=b)}. How it is done: - 1. The method finds the left part of the equalities to be built. It will - be the same for all equalities. It is either: - a. A constant if there is any - b. A first element in the multiple equality that satisfies - checker function + 1. If there is a constant member c the first non-constant member x for + which the function checker returns true is taken and an item for + the equality x=c is created. When constructing the equality item + the left part of the equality is always taken as a clone of x while + the right part is taken as a clone of c only if clone_const == true. - For the example 1 the left element is field 'x'. - For the example 2 it is constant '1'. + 2. After this all equalities of the form x=a (where x designates the first + non-constant member for which checker returns true and a is some other + such member of the multiplle equality) are created. When constructing + an equality item both its parts are taken as clones of x and a. - 2. If the left element is found the rest elements of the multiple equality - are checked with the checker function if they can be right parts - of equalities. - If the element can be a right part of the equality, equality is built. - It is built with the left part element found at the step 1 and - the right part element found at this step (step 2). - - Suppose for the example above that both 'a' and 'b' fields can be used - to build equalities: + Suppose in the examples above that for 'x', 'a', and 'b' the function + checker returns true. Example 1: - for 'a' field (x=a) is built - for 'b' field (x=b) is built + the equality (x=a) is built + the equality (x=b) is built Example 2: - for 'a' field (1=a) is built - for 'b' field (1=b) is built + the equality (a=1) is built + the equality (a=b) is built 3. As a result we get a set of equalities built with the elements of this multiple equality. They are saved in the equality list. @@ -7458,15 +7454,17 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) {(x=a),(x=b)} Example 2: - {(1=a),(1=b)} + {(a=1),(a=b)} @note This method is called for condition pushdown into materialized derived table/view, and IN subquery, and pushdown from HAVING into WHERE. When it is called for pushdown from HAVING the empty checker is passed. - It happens because elements of this multiple equality don't need to be - checked if they can be used to build equalities. There are no elements - that can't be used to build equalities. + This is because in this case the elements of the multiple equality don't + need to be checked if they can be used to build equalities: either all + equalities can be pushed or none of them can be pushed. + When the function is called for pushdown from HAVING the value of the + parameter clone_const is always false. In other cases it's always true. @retval true if an error occurs @retval false otherwise @@ -7475,24 +7473,42 @@ Item_equal::excl_dep_on_grouping_fields(st_select_lex *sel) bool Item_equal::create_pushable_equalities(THD *thd, List<Item> *equalities, Pushdown_checker checker, - uchar *arg) + uchar *arg, + bool clone_const) { Item *item; + Item *left_item= NULL; + Item *right_item = get_const(); Item_equal_fields_iterator it(*this); - Item *left_item = get_const(); - if (!left_item) + + while ((item=it++)) { - while ((item=it++)) - { - left_item= item; - if (checker && !((item->*checker) (arg))) - continue; - break; - } + left_item= item; + if (checker && !((item->*checker) (arg))) + continue; + break; } + if (!left_item) return false; + if (right_item) + { + Item_func_eq *eq= 0; + Item *left_item_clone= left_item->build_clone(thd); + Item *right_item_clone= !clone_const ? + right_item : right_item->build_clone(thd); + if (!left_item_clone || !right_item_clone) + return true; + eq= new (thd->mem_root) Item_func_eq(thd, + left_item_clone, + right_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) + return true; + if (!clone_const) + right_item->set_extraction_flag(IMMUTABLE_FL); + } + while ((item=it++)) { if (checker && !((item->*checker) (arg))) @@ -7500,15 +7516,14 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item_func_eq *eq= 0; Item *left_item_clone= left_item->build_clone(thd); Item *right_item_clone= item->build_clone(thd); - if (left_item_clone && right_item_clone) - { - left_item_clone->set_item_equal(NULL); - right_item_clone->set_item_equal(NULL); - eq= new (thd->mem_root) Item_func_eq(thd, - right_item_clone, - left_item_clone); - } - if (eq && equalities->push_back(eq, thd->mem_root)) + if (!(left_item_clone && right_item_clone)) + return true; + left_item_clone->set_item_equal(NULL); + right_item_clone->set_item_equal(NULL); + eq= new (thd->mem_root) Item_func_eq(thd, + right_item_clone, + left_item_clone); + if (!eq || equalities->push_back(eq, thd->mem_root)) return true; } return false; @@ -7533,7 +7548,7 @@ bool Item_equal::create_pushable_equalities(THD *thd, Item *Item_equal::multiple_equality_transformer(THD *thd, uchar *arg) { List<Item> equalities; - if (create_pushable_equalities(thd, &equalities, 0, 0)) + if (create_pushable_equalities(thd, &equalities, 0, 0, false)) return 0; switch (equalities.elements) diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 16bb53c..ea34679 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -7988,7 +7988,7 @@ st_select_lex::check_cond_extraction_for_grouping_fields(THD *thd, Item *cond) } else { - int fl= cond->excl_dep_on_grouping_fields(this) ? + int fl= cond->excl_dep_on_grouping_fields(this) && !cond->is_expensive() ? FULL_EXTRACTION_FL : NO_EXTRACTION_FL; cond->set_extraction_flag(fl); } @@ -9819,7 +9819,7 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) { List_iterator<Item> li(*((Item_cond*) result)->argument_list()); Item *item; - while ((item=li++)) + while ((item= li++)) { if (attach_to_conds.push_back(item, thd->mem_root)) return true; @@ -9839,8 +9839,13 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) */ if (cond->type() != Item::COND_ITEM) return false; + if (((Item_cond *)cond)->functype() != Item_cond::COND_AND_FUNC) { + /* + cond is not a conjunctive formula and it cannot be pushed into WHERE. + Try to extract a formula that can be pushed. + */ Item *fix= cond->build_pushable_cond(thd, 0, 0); if (!fix) return false; @@ -9860,7 +9865,6 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) Item *result= item->transform(thd, &Item::multiple_equality_transformer, (uchar *)item); - if (!result) return true; if (result->type() == Item::COND_ITEM && @@ -10188,8 +10192,8 @@ Item *st_select_lex::pushdown_from_having_into_where(THD *thd, Item *having) &Item::field_transformer_for_having_pushdown, (uchar *)this); - if (item->walk(&Item:: cleanup_processor, 0, STOP_PTR) || - item->fix_fields(thd, NULL)) + if (item->walk(&Item::cleanup_excluding_immutables_processor, 0, STOP_PTR) + || item->fix_fields(thd, NULL)) { attach_to_conds.empty(); goto exit;

1 0

[Commits] e8d3583fc88: MDEV-18514: Assertion `!writer.checksum_len || writer.remains == 0' failed
by sujatha 31 Dec '19

31 Dec '19

revision-id: e8d3583fc88ea8c08a94aceba5a21bd5e367c454 (mariadb-10.2.30-39-ge8d3583fc88) parent(s): 16bce0f6fe6bcad0091dc45a97a8ac7b33fe9d44 author: Sujatha committer: Sujatha timestamp: 2019-12-31 11:49:42 +0530 message: MDEV-18514: Assertion `!writer.checksum_len || writer.remains == 0' failed Analysis: ======== 'max_binlog_cache_size' is configured and a huge transaction is executed. When the transaction specific events size exceeds 'max_binlog_cache_size' the event cannot be written to the binary log cache and cache write error is raised. Upon cache write error the statement is rolled back and the transaction cache should be truncated to a previous statement specific position. The truncate operation should reset the cache to earlier valid positions and flush the new changes. Even though the flush is successful the cache write error is still in marked state. The truncate code interprets the cache write error as cache flush failure and returns abruptly without modifying the write cache parameters. Hence cache is in a invalid state. When a COMMIT statement is executed in this session it tries to flush the contents of transaction cache to binary log. Since cache has partial events the cache write operation will report 'writer.remains' assert. Fix: === During the rollback operation truncate the cache and flush the cache contents. If truncation is successful clear the cache write error. --- .../suite/rpl/r/rpl_binlog_rollback_cleanup.result | 9 +++++ .../suite/rpl/t/rpl_binlog_rollback_cleanup.test | 46 ++++++++++++++++++++++ mysys/mf_iocache.c | 2 + 3 files changed, 57 insertions(+) diff --git a/mysql-test/suite/rpl/r/rpl_binlog_rollback_cleanup.result b/mysql-test/suite/rpl/r/rpl_binlog_rollback_cleanup.result new file mode 100644 index 00000000000..a677cbfecf6 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_binlog_rollback_cleanup.result @@ -0,0 +1,9 @@ +include/master-slave.inc +[connection master] +connection master; +SET GLOBAL max_binlog_cache_size = 65536; +CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=INNODB; +ERROR HY000: Multi-statement transaction required more than 'max_binlog_cache_size' bytes of storage; increase this mysqld variable and try again +SET GLOBAL max_binlog_cache_size= ORIGINAL_VALUE; +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_binlog_rollback_cleanup.test b/mysql-test/suite/rpl/t/rpl_binlog_rollback_cleanup.test new file mode 100644 index 00000000000..ed4d713f626 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_binlog_rollback_cleanup.test @@ -0,0 +1,46 @@ +# ==== Purpose ==== +# +# Test verifies that when flushing an event to binary log fails the transaction +# is successfully rolled back and following COMMIT command doesn't report any +# assert. +# +# ==== Implementation ==== +# +# Steps: +# 0 - SET max_binlog_cache_size=64K +# 1 - Create an Innodb table and insert required amount of data. Execute an +# UPDATE operation which generates a big update event whose size exceeds +# max_binlog_cache_size. +# 2 - Wait for error 1197. Execute COMMIT command. +# 3 - COMMIT should be successful. +# +# ==== References ==== +# +# MDEV-18514: Assertion `!writer.checksum_len || writer.remains == 0' failed +# +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc +--connection master +let $old_max_binlog_cache_size= query_get_value(SHOW VARIABLES LIKE "max_binlog_cache_size", Value, 1); +SET GLOBAL max_binlog_cache_size = 65536; +CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=INNODB; +let $data = `select concat('"', repeat('a',6000), '"')`; +let $data1 = `select concat('"', repeat('b',6000), '"')`; +--disable_query_log +eval INSERT INTO t1 (a, data) VALUES (1, CONCAT($data, $data)); +eval INSERT INTO t1 (a, data) VALUES (2, CONCAT($data, $data)); +eval INSERT INTO t1 (a, data) VALUES (3, CONCAT($data, $data)); +eval INSERT INTO t1 (a, data) VALUES (4, CONCAT($data, $data)); +eval INSERT INTO t1 (a, data) VALUES (5, CONCAT($data, $data)); +START TRANSACTION; +--error ER_TRANS_CACHE_FULL +eval UPDATE t1 SET data=$data1; +COMMIT; +--enable_query_log + +--replace_result $old_max_binlog_cache_size ORIGINAL_VALUE +--eval SET GLOBAL max_binlog_cache_size= $old_max_binlog_cache_size +DROP TABLE t1; + +--source include/rpl_end.inc diff --git a/mysys/mf_iocache.c b/mysys/mf_iocache.c index d7689e204b6..6196e65b9de 100644 --- a/mysys/mf_iocache.c +++ b/mysys/mf_iocache.c @@ -1949,6 +1949,8 @@ int my_b_flush_io_cache(IO_CACHE *info, int need_append_buffer_lock) int res= info->write_function(info, info->write_buffer, length); if (res) DBUG_RETURN(res); + else + info->error= 0; set_if_bigger(info->end_of_file, info->pos_in_file); }

1 0

[Commits] d3a2c173b63: Big Test added for sorting
by Varun 30 Dec '19

30 Dec '19

revision-id: d3a2c173b63e784513e67502eb91345f7620bd7d (mariadb-10.4.11-18-gd3a2c173b63) parent(s): 59d4f2a373a7960a533e653877ab69a97e91444a author: Varun Gupta committer: Varun Gupta timestamp: 2019-12-31 03:18:09 +0530 message: Big Test added for sorting --- mysql-test/main/order_by_pack_big.result | 170 +++++++++++++++++++++++++++++++ mysql-test/main/order_by_pack_big.test | 81 +++++++++++++++ 2 files changed, 251 insertions(+) diff --git a/mysql-test/main/order_by_pack_big.result b/mysql-test/main/order_by_pack_big.result new file mode 100644 index 00000000000..b1b7b7e5940 --- /dev/null +++ b/mysql-test/main/order_by_pack_big.result @@ -0,0 +1,170 @@ +set @save_rand_seed1= @@RAND_SEED1; +set @save_rand_seed2= @@RAND_SEED2; +set @@RAND_SEED1=810763568, @@RAND_SEED2=600681772; +create table t1(a int); +insert into t1 select seq from seq_1_to_10000 order by rand(); +# +# function f1 has parameters mean(peak) and std_dev (standard deviation) +# for a normal distribution +# +CREATE FUNCTION f1(mean DOUBLE, std_dev DOUBLE) RETURNS DOUBLE +BEGIN +set @z= (rand() + rand() + rand() + rand() + rand() + rand() + +rand() + rand() + rand() + rand() + rand() + rand() - 6); +set @z= std_dev*@z + mean; +return @z; +END| +CREATE function f2(len INT) RETURNS varchar(256) +BEGIN +DECLARE str VARCHAR(256) DEFAULT ''; +DECLARE x INT DEFAULT 0; +WHILE (len > 0 AND len < 256) DO +SET x =round(rand()*25); +SET str= CONCAT(str, CHAR(65 + x)); +SET len= len-1; +END WHILE; +RETURN str; +END| +CREATE function f3(mean DOUBLE, std_dev DOUBLE, min_val INT) RETURNS INT +BEGIN +DECLARE r DOUBLE DEFAULT 0; +WHILE 1=1 DO +set r= f1(mean, std_dev); +IF (r >= min_val) THEN +RETURN round(r); +end if; +END WHILE; +RETURN 0; +END| +create table t2 (id INT NOT NULL, a INT, b int); +insert into t2 select a, f3(12, 8.667, 0), f3(32, 16, 0) from t1; +CREATE TABLE t3( +id INT NOT NULL, +names VARCHAR(64), +address VARCHAR(132), +PRIMARY KEY (id) +); +insert into t3 select id, f2(a) , f2(b) from t2; +set sort_buffer_size=262144*10; +flush status; +select id, +MD5(group_concat(substring(names,1,3), substring(address,1,3))) +FROM t3 +GROUP BY id DIV 100 +ORDER BY id; +id MD5(group_concat(substring(names,1,3), substring(address,1,3))) +10 4e2909ba6af73a6f2331c332272ebe6f +149 2591aa134a03c0baedb1f2e09d37fc01 +232 779fe09e8677c88c17dab1c2c5c54a9e +311 6f66733f6f2fee55853a4081dac362bf +430 b7431e4e30f7ce214350649a9c1877fa +502 5a39b69003d083e81ffc5ff7d1840ae0 +665 796aafa79e47c3f99a2f83c2fe87e9af +719 1adaa36769a3ec1aaf61cc67ee035bf2 +883 558686327dba2531104dc32497f13806 +942 68c2c6be36ad81da00f875e05678ac1a +1007 c9fe7a2a3b1e77f3512afa96edcdbd0f +1134 f51a946d3bcefb2ef0bb9a816654993f +1226 968710fb0ee891bf5efedac64daa7f59 +1367 e84b491129560c4a31176ec17c7e4651 +1486 f13314fa07ab1786256544206f552126 +1502 723fa7da3db0cfc61bce43af13bd7733 +1674 9296d3b0d959e7c7bfb0536c0a680003 +1781 dd9dc3ee5356e90a7279d2a7e09ab067 +1803 a0787eb53c2ddfe4dc501cbd62cd6ef5 +1969 d0d2d4c9a33b5e7c7ba517b21bbb225c +2087 0491191a92ecef7813b491c651c3017a +2111 be7737ae27e517dee4d2c2490dd08fe5 +2268 13387d22efac2835387acb5fdf466847 +2328 0da4eaaa6d40790e83426adc88c6b867 +2416 c6376f9b85a6247e4bb4b7fed7019654 +2599 07c0494dc58e02c9e4d476ed4e828999 +2602 3f2b2cd46fdbcb9f4de5d22af1ddf332 +2777 cf080c656cf2c72c6c40df9522c4a444 +2858 dd6f033f165c3764e5eee748b6a4b306 +2922 d69118e72d77dc35d437a23bc620600a +3027 d7b6c3a6cd712c99fa428d40d7d14e8f +3131 8765144e43704034b8bde8195c3bd566 +3220 4da111ce557f3c27ed0413f5ced46e66 +3331 c4b80698fd39a7124e5bbac98d949980 +3452 db6e2ab7247a49a5a03ae9eb5d197c50 +3519 271a797d3251ea00339f72d18ccb6d2f +3680 7c04e35481d3d64878330db84158bb67 +3799 15febc7ee00f06dbf96a961d90840571 +3876 936394440bf45ba95fcb237c106f0ad4 +3937 1468b953f59d3d9124646cb9a62a066c +4029 a2d060975fe98313cc4d6d0967134862 +4158 3ef575890dc439f165a6c0e7cecef3a0 +4291 ad24580ca5589c22e66ee9b2507bffe2 +4355 469e098eb29d367bf52e8dcb626af560 +4461 985a0571305b940cf9cdddc0bd992868 +4544 b596b41cced9a72635ebb621990a9884 +4646 530f8884df8f69e89a422dccf15dde4e +4714 e4c39cbe81b717e5dc55c3d8a5f958bd +4848 0441809790e48eae3e3451982d6e9685 +4935 5bf7c598b37c14c1c667c7a3e765ef4b +5014 52af0e4bcdcaa9798efa5208fc14160f +5184 fb35e60e92eb7d77d24d74fca72f1d74 +5219 3b7efa9a63a7c2abc6242706bd4e178b +5349 e34bcfd71d15658df22d985d7cfd4f46 +5400 8567ac3ba9b56908b1df46b72b44e91d +5507 b437451c7e0d0b0be72f7bea8cbad4ff +5672 071a6b393989a88074bf31f780d7d6ee +5725 eced78afcf11ac492c2e14682ca097dd +5849 376cd2d795fb1cc25ea9832e37b193fd +5941 c25cbc66d609b9beee3c948cb0339f74 +6064 4b0eaec74cb4c0234b30373493749360 +6135 a5108f050da716c3f2b030587c1ce615 +6289 b19351d2e4e58a77f1f33f15c90c6181 +6329 b213caf63af61795ac76e7c56d1d1314 +6404 a6128a481c3c23141b200f0693bba3a7 +6580 06dca934fccedb081f18d418fc7a3909 +6653 20dd771bd5239beb009b81853f5adc72 +6773 cb107d95104e7ed5f9796a3fc8103251 +6822 ed49d107938a7fb5058d8e6f719b97fb +6938 a638345003ca5ee6ea677446193c4af2 +7085 1a5ed3ccde9927cb1f4dfc8b2b71f25f +7180 7275a16515755a6b789da7c2fdb2f096 +7290 aa916ee2a7b4f6aff0592309e890d6c0 +7390 040861544798bccfaebec94c719e2fa9 +7449 58ddd09159a92cd3fee0c241b8629a8c +7589 3244004b56a66402cde3c60a41d99dfc +7686 960d350f442e03aa745d4740b1dee8f1 +7713 e43df84b0b6a40d26a4e36bb371af6c1 +7844 b3ac24da8fdba43ba00ad2772cfeedd8 +7935 a7d07f684c0c18946695a5fa369139f0 +8059 81523df1da525bb0e5490aec44809552 +8153 a34469e694de9c3e3d7d2cb171df691a +8245 3bdcc5556a474a7a67be2134804987a8 +8310 10ed2c9f70315df533c94d5a79200452 +8432 3f7273962e617832452163b98058791a +8596 2eec8a128e031b31e890fd545aef9c20 +8647 1ae5260b3db571fcfdf36e17b9286d73 +8742 da8e83a01b7e17d4b28de7275b83ccbc +8856 426253c28af8a7364a0c4795355590f9 +8967 1cad1040530e52c64e4f5ba5bf640fdd +9057 7c265622653013e1bfa47dc4b1f3289f +9148 dd25afe1e73889c5810e2fa91c31ca6c +9275 5e8b0a1ecfde14c6fa8d4cba12aa8ac1 +9311 b1f7571deb95309671533b657b5392ab +9488 324bbfc66eaf7254c7b1d99b23cd33f0 +9518 5f514525711e5c42e067776a3139d011 +9696 10f135781fe4bfffc5b1feb18818e592 +9752 87c432d39312dcc756ea49ae3a63bfe1 +9887 2e8c5ee85ee68afdf25985e9b1c01bd6 +9903 4e4c6529cb69a85c095381d0991dedcf +10000 a849cf46ea623ee01daa24e77a05d671 +show status like '%sort%'; +Variable_name Value +Sort_merge_passes 0 +Sort_priority_queue_sorts 0 +Sort_range 0 +Sort_rows 10101 +Sort_scan 2 +set sort_buffer_size=default; +set @@RAND_SEED1= @save_rand_seed1; +set @@RAND_SEED2= @save_rand_seed2; +drop function f1; +drop function f2; +drop function f3; +drop table t1, t2, t3; diff --git a/mysql-test/main/order_by_pack_big.test b/mysql-test/main/order_by_pack_big.test new file mode 100644 index 00000000000..3840e023e2c --- /dev/null +++ b/mysql-test/main/order_by_pack_big.test @@ -0,0 +1,81 @@ +--source include/big_test.inc +--source include/have_sequence.inc +--source include/have_64bit.inc + +set @save_rand_seed1= @@RAND_SEED1; +set @save_rand_seed2= @@RAND_SEED2; +set @@RAND_SEED1=810763568, @@RAND_SEED2=600681772; + +create table t1(a int); +insert into t1 select seq from seq_1_to_10000 order by rand(); +delimiter |; + +--echo # +--echo # function f1 has parameters mean(peak) and std_dev (standard deviation) +--echo # for a normal distribution +--echo # + +CREATE FUNCTION f1(mean DOUBLE, std_dev DOUBLE) RETURNS DOUBLE +BEGIN + set @z= (rand() + rand() + rand() + rand() + rand() + rand() + + rand() + rand() + rand() + rand() + rand() + rand() - 6); + set @z= std_dev*@z + mean; + return @z; +END| + +CREATE function f2(len INT) RETURNS varchar(256) +BEGIN + DECLARE str VARCHAR(256) DEFAULT ''; + DECLARE x INT DEFAULT 0; + WHILE (len > 0 AND len < 256) DO + SET x =round(rand()*25); + SET str= CONCAT(str, CHAR(65 + x)); + SET len= len-1; + END WHILE; +RETURN str; +END| + +CREATE function f3(mean DOUBLE, std_dev DOUBLE, min_val INT) RETURNS INT +BEGIN + DECLARE r DOUBLE DEFAULT 0; + WHILE 1=1 DO + set r= f1(mean, std_dev); + IF (r >= min_val) THEN + RETURN round(r); + end if; + END WHILE; + RETURN 0; +END| + +delimiter ;| + + +create table t2 (id INT NOT NULL, a INT, b int); +insert into t2 select a, f3(12, 8.667, 0), f3(32, 16, 0) from t1; + +CREATE TABLE t3( + id INT NOT NULL, + names VARCHAR(64), + address VARCHAR(132), + PRIMARY KEY (id) +); + +insert into t3 select id, f2(a) , f2(b) from t2; + +set sort_buffer_size=262144*10; +flush status; +select id, + MD5(group_concat(substring(names,1,3), substring(address,1,3))) +FROM t3 +GROUP BY id DIV 100 +ORDER BY id; +show status like '%sort%'; +set sort_buffer_size=default; + +set @@RAND_SEED1= @save_rand_seed1; +set @@RAND_SEED2= @save_rand_seed2; + +drop function f1; +drop function f2; +drop function f3; +drop table t1, t2, t3;

1 0

[Commits] d855595deab: MDEV-21263: Allow packed values of non-sorted fields in the sort buffer
by Varun 26 Dec '19

26 Dec '19

revision-id: d855595deab0be296b6e88059de2e8a2eda27d07 (mariadb-10.5.0-68-gd855595deab) parent(s): 89633995e4962a7ad4a241cdf62ee637990d6787 author: Varun Gupta committer: Varun Gupta timestamp: 2019-12-26 11:58:55 +0530 message: MDEV-21263: Allow packed values of non-sorted fields in the sort buffer This task deals with packing the non-sorted fields (or addon fields). This would lead to efficient usage of the memory allocated for the sort buffer. The changes brought by this feature are 1) Sort buffers would have records of variable length 2) Each record in the sort buffer would be stored like <sort_key1><sort_key2>....<addon_length><null_bytes><field1><field2>.... addon_length is the extra bytes that are required to store the variable length of addon field across different records. 3) Changes in rr_unpack_from_buffer and rr_from_tempfile to take into account the variable length of records. Ported WL#1509 Pack values of non-sorted fields in the sort buffer from MySQL by Tor Didriksen --- mysql-test/main/order_by_pack_big.result | 153 +++++ mysql-test/main/order_by_pack_big.test | 62 ++ sql/bounded_queue.h | 4 +- sql/field.h | 2 + sql/filesort.cc | 700 +++++++++++++-------- sql/filesort.h | 83 ++- sql/filesort_utils.cc | 64 +- sql/filesort_utils.h | 214 ++++++- sql/records.cc | 141 ++++- sql/records.h | 14 +- sql/sql_array.h | 4 + sql/sql_select.cc | 4 +- sql/sql_sort.h | 291 ++++++++- sql/uniques.cc | 80 ++- sql/uniques.h | 2 +- .../mysql-test/connect/r/mysql_index.result | 4 +- .../connect/mysql-test/connect/t/mysql_index.test | 2 +- 17 files changed, 1390 insertions(+), 434 deletions(-) diff --git a/mysql-test/main/order_by_pack_big.result b/mysql-test/main/order_by_pack_big.result new file mode 100644 index 00000000000..a65e8f61579 --- /dev/null +++ b/mysql-test/main/order_by_pack_big.result @@ -0,0 +1,153 @@ +set @save_rand_seed1= @@RAND_SEED1; +set @save_rand_seed2= @@RAND_SEED2; +set @@RAND_SEED1=810763568, @@RAND_SEED2=600681772; +create table t1(a int); +insert into t1 select seq from seq_1_to_10000 order by rand(); +CREATE FUNCTION f1(median INT) RETURNS DOUBLE +BEGIN +set @z = (rand() + rand() + rand() + rand() + rand() + rand())/6; +set @z = @z*median*2+1; +return round(@z); +END| +CREATE function f2(len INT) RETURNS varchar(256) +BEGIN +DECLARE str VARCHAR(256) DEFAULT ''; +DECLARE x INT DEFAULT 0; +WHILE (len > 0 AND len < 256) DO +SET x =round(rand()*25); +SET str= CONCAT(str, CHAR(65 + x)); +SET len= len-1; +END WHILE; +RETURN str; +END| +create table t3 (id INT NOT NULL, a INT, b int); +insert into t3 select a, f1(12), f1(32) from t1; +CREATE TABLE t2( +id INT NOT NULL, +names VARCHAR(64), +address VARCHAR(132), +PRIMARY KEY (id) +); +insert into t2 select a, f2(f1(12)) , f2(f1(32)) from t1; +set sort_buffer_size=262144*10; +flush status; +select id, +MD5(group_concat(substring(names,1,3), substring(address,1,3))) +FROM t2 +GROUP BY id DIV 100 +ORDER BY id; +id MD5(group_concat(substring(names,1,3), substring(address,1,3))) +10 b253ed13e64cf733323d27afc92d11b9 +149 43ea485e548da5dd3fcd8c4906dccd7e +232 6ccc7fc342e42192ccdfbb77e85fa433 +311 90f94659c39641a213d3744bbffc41e0 +430 a53bead2e13ee4bddd5c2fb66c04c917 +502 419c219a887215fe89226ed9226bfa1e +665 1e7133708a3e1bc43f37d7173996743b +719 9a6863e2c59ea8b03395f8ff8df49412 +883 5ebe88422a3a7a4df6048471b1371392 +942 51747d9e4b077ed1426cc23e76865cf0 +1007 ba48b7a8d9ce563b2ee39a52bd1283e5 +1134 800292609f2c2a3a794e3afe20d17e20 +1226 c72aaf8bbd16c026d0ba70c15e2ba31d +1367 4b6b782916e30ac4f87353faa63e5784 +1486 cd4ba93263a8f1b90b16017c2a12581f +1502 08be1a9c9d0fe26afa885dece8dff600 +1674 5ee5624374baf6a2fdb69a760a9c1e3c +1781 a569b76cce1a35bcebd4629700fae8da +1803 0f882c91efad7865d9a808a8905bdcd2 +1969 3bb29bf3c1ffa7e33825e426ba3a67fe +2087 4c8ded95fd37400de735ebd6613f28d1 +2111 92f1941edd43dd254471c758dcec0125 +2268 e577a7e2de58d4097693fec26179564d +2328 c74d953831057abf57019a847af38982 +2416 185c279928fe6860d70635be36e59f81 +2599 d0e84deb33d2b299c914b3d0ffbf56c8 +2602 bba2e5c462eb682aeaf4d423fe088f88 +2777 8acf4418303155237e2e45973a95e03b +2858 4e503f79070142fcc39161cfcee3d5cc +2922 54c4420bd27edd931db9de43dc78e304 +3027 8c7edeec29069b0369024f771a208ccf +3131 b72b4098fa4cc8b7ca9e44ebf49bfdcd +3220 1447ca382f80c823aa769692af4c79a4 +3331 37c48ec7c7f6d6e402431d4ce9162ca3 +3452 54396ef321b22670fdf76372c102278f +3519 331a7e1f8f758db2f83c35a7dd6169de +3680 909128ecfc539d004852a1b085d97b0a +3799 4f36d7d8844cd0ef2d09c5ecb4a0e147 +3876 42038816d752bf58f15fdaab41326c85 +3937 b79796f21d78da6908b06a393deae228 +4029 4225c1b7b0242f9f52b373ff08bdaa35 +4158 3ded3bb37f1d6beaeff4b144c44f6283 +4291 09c7086bada0918fb7b49cb5d18cbc63 +4355 9bc74fee3974ac0a37d5aeb47b3e8fe7 +4461 a208fdfd27499fb25e1e4fb17474ad2d +4544 985d5096cb3e7721bf2bda5b3652aa6e +4646 4111a94e90540d2869d1a4443048ff5b +4714 a50de2a7c76405fd7e9ee4a245b7443e +4848 4f3d9f6c2214d1dbfcbeb52d24db4803 +4935 196434d066598e31480970cef6858992 +5014 64a725bcf6bf00ea93a974cc5e67faed +5184 3b13c7d69d2cf79295a99517c37917e5 +5219 9ea5ef3ad046b49a696bcbb1f6b46073 +5349 cb541d0cc1deec0309330d23384e6b73 +5400 d13a8f18ff97c555f94016765084ffa1 +5507 f22f063f95f49d447fe668612c6df270 +5672 8d4d95cb5e75e377b0316fd0ebda84f0 +5725 f93deb9bf142a233c487441989500ad2 +5849 1a3f23db800626853f20df9912eda5bb +5941 202f31698078ca21ba5ea37fa20c0a12 +6064 1b3a59f5cb65c1710253ea455aeb6e65 +6135 d13e27505091f5a2e1b645ce77daa2a1 +6289 c92136d3d26acd47d132ddfc78adc729 +6329 f3ea174f4de598bc6d7d2d2c464acb14 +6404 54110ca947fa2d23c9735b7e5a5a50cf +6580 9478efd805dc2f2e3f1e770f3097d1d1 +6653 d5267e02f46dc3d2f169ea3f578371e8 +6773 f18e4f7005b81d1a950d4a1bba8b1534 +6822 9f8860eaf35ba68cfeb0ad6a78a2cc56 +6938 78e2b08e77fb6d969550b7eb1967da28 +7085 bbbd509f22273997f5461c10b17075d4 +7180 9e18b5a59366df9ad7a62a4f424153ed +7290 0a60c3d6661499de48d2ced87409e9bb +7390 93e875e3c9d690e1d01198c0c44c3cb2 +7449 c98c9cb4d00db6c707e284c0f318c9fc +7589 e2058c36596b0d23b4b1f2ca8591e6be +7686 0f182c5acb930902f62cdcf428f4745f +7713 eebff08db371b43703acf8aba8e483b1 +7844 3036d395f077b96f89e1b7bc5986ceb6 +7935 3b67bd296f31c17da7074dda34590d7b +8059 d7ff3396e0a4739f867438d60e93bac8 +8153 6e8a00cd8f006010c295195a3c7eef01 +8245 e65335846dde70d2e22687026b03a35f +8310 d1959cf02010be41a5294c86acd1fb07 +8432 db80164eee1baf3c95c2d34796ca8d4b +8596 f4774e3f3aa83c5ff8d4053df36cb3a6 +8647 00a98557e738bb097ab4de083507479e +8742 407e13848129bacb38edd4a12ce696cf +8856 18fef961b5144c669a43eb010d02e972 +8967 741b7ff21a3e73cb47092f43d85e8c6c +9057 ace84e526e26f0497f3621192d0e4a84 +9148 a2268d21fcdfafc04e22422cfff90bc8 +9275 811cc7fc2a15d64fe3c3b6caf84b92c7 +9311 15d529de7dc6536040dc2a1d8fd25013 +9488 1c9331758eb2a3675891b8af982d79c1 +9518 1cbde8577db1d61225037045d9706666 +9696 7a6fcd67a46a3a5a5ed2efd0848c9e92 +9752 b0d433f38ca8f67f079d87789ad40618 +9887 b125b717470a8260dfdaf0b90a1b9d70 +9903 602cdc924ea991d73014a15b5404e1d8 +10000 d450f3b4dc99fa4f0ed6046e770ba3ee +show status like '%sort%'; +Variable_name Value +Sort_merge_passes 0 +Sort_priority_queue_sorts 0 +Sort_range 0 +Sort_rows 10101 +Sort_scan 2 +set sort_buffer_size=default; +set @@RAND_SEED1= @save_rand_seed1; +set @@RAND_SEED2= @save_rand_seed2; +drop function f1; +drop function f2; +drop table t1,t2,t3; diff --git a/mysql-test/main/order_by_pack_big.test b/mysql-test/main/order_by_pack_big.test new file mode 100644 index 00000000000..d3072800217 --- /dev/null +++ b/mysql-test/main/order_by_pack_big.test @@ -0,0 +1,62 @@ +--source include/big_test.inc +--source include/have_sequence.inc +--source include/have_64bit.inc + +set @save_rand_seed1= @@RAND_SEED1; +set @save_rand_seed2= @@RAND_SEED2; +set @@RAND_SEED1=810763568, @@RAND_SEED2=600681772; + +create table t1(a int); +insert into t1 select seq from seq_1_to_10000 order by rand(); + +delimiter |; + +CREATE FUNCTION f1(median INT) RETURNS DOUBLE +BEGIN + set @z = (rand() + rand() + rand() + rand() + rand() + rand())/6; + set @z = @z*median*2+1; + return round(@z); +END| + +CREATE function f2(len INT) RETURNS varchar(256) +BEGIN + DECLARE str VARCHAR(256) DEFAULT ''; + DECLARE x INT DEFAULT 0; + WHILE (len > 0 AND len < 256) DO + SET x =round(rand()*25); + SET str= CONCAT(str, CHAR(65 + x)); + SET len= len-1; + END WHILE; +RETURN str; +END| + +delimiter ;| + +create table t3 (id INT NOT NULL, a INT, b int); +insert into t3 select a, f1(12), f1(32) from t1; + +CREATE TABLE t2( + id INT NOT NULL, + names VARCHAR(64), + address VARCHAR(132), + PRIMARY KEY (id) +); + +insert into t2 select a, f2(f1(12)) , f2(f1(32)) from t1; + +set sort_buffer_size=262144*10; +flush status; +select id, + MD5(group_concat(substring(names,1,3), substring(address,1,3))) +FROM t2 +GROUP BY id DIV 100 +ORDER BY id; +show status like '%sort%'; +set sort_buffer_size=default; + +set @@RAND_SEED1= @save_rand_seed1; +set @@RAND_SEED2= @save_rand_seed2; + +drop function f1; +drop function f2; +drop table t1,t2,t3; diff --git a/sql/bounded_queue.h b/sql/bounded_queue.h index fd733caa019..cd710d835aa 100644 --- a/sql/bounded_queue.h +++ b/sql/bounded_queue.h @@ -57,7 +57,7 @@ class Bounded_queue @param to Where to put the key. @param from The input data. */ - typedef void (*keymaker_function)(Sort_param *param, + typedef uint (*keymaker_function)(Sort_param *param, Key_type *to, Element_type *from); @@ -181,7 +181,7 @@ void Bounded_queue<Element_type, Key_type>::push(Element_type *element) { // Replace top element with new key, and re-order the queue. Key_type **pq_top= reinterpret_cast<Key_type **>(queue_top(&m_queue)); - (*m_keymaker)(m_sort_param, *pq_top, element); + (void)(*m_keymaker)(m_sort_param, *pq_top, element); queue_replace_top(&m_queue); } else { // Insert new key into the queue. diff --git a/sql/field.h b/sql/field.h index 911fe430371..3e45fe774b3 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1527,6 +1527,7 @@ class Field: public Value_source { return length;} virtual uint max_packed_col_length(uint max_length) { return max_length;} + virtual bool is_packable() { return false; } uint offset(const uchar *record) const { @@ -2139,6 +2140,7 @@ class Field_longstr :public Field_str bool can_optimize_range(const Item_bool_func *cond, const Item *item, bool is_eq_func) const; + bool is_packable() { return true; } }; /* base class for float and double and decimal (old one) */ diff --git a/sql/filesort.cc b/sql/filesort.cc index df6e1eb9104..d93ae3b595a 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -48,17 +48,17 @@ static ha_rows find_all_keys(THD *thd, Sort_param *param, SQL_SELECT *select, ha_rows *found_rows); static bool write_keys(Sort_param *param, SORT_INFO *fs_info, uint count, IO_CACHE *buffer_file, IO_CACHE *tempfile); -static void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos); +static uint make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos); static void register_used_fields(Sort_param *param); static bool save_index(Sort_param *param, uint count, SORT_INFO *table_sort); static uint suffix_length(ulong string_length); static uint sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length, - bool *multi_byte_charset); -static SORT_ADDON_FIELD *get_addon_fields(TABLE *table, uint sortlength, - LEX_STRING *addon_buf); -static void unpack_addon_fields(struct st_sort_addon_field *addon_field, - uchar *buff, uchar *buff_end); + bool *multi_byte_charset); +static Addon_fields *get_addon_fields(TABLE *table, uint sortlength, + uint *addon_length, + uint *m_packable_length); + static bool check_if_pq_applicable(Sort_param *param, SORT_INFO *info, TABLE *table, ha_rows records, size_t memory_available); @@ -66,7 +66,7 @@ static bool check_if_pq_applicable(Sort_param *param, SORT_INFO *info, void Sort_param::init_for_filesort(uint sortlen, TABLE *table, ha_rows maxrows, bool sort_positions) { - DBUG_ASSERT(addon_field == 0 && addon_buf.length == 0); + DBUG_ASSERT(addon_fields == NULL); sort_length= sortlen; ref_length= table->file->ref_length; @@ -77,12 +77,13 @@ void Sort_param::init_for_filesort(uint sortlen, TABLE *table, Get the descriptors of all fields whose values are appended to sorted fields and get its total length in addon_buf.length */ - addon_field= get_addon_fields(table, sort_length, &addon_buf); + addon_fields= get_addon_fields(table, sort_length, &addon_length, + &m_packable_length); } - if (addon_field) + if (using_addon_fields()) { - DBUG_ASSERT(addon_buf.length < UINT_MAX32); - res_length= (uint)addon_buf.length; + DBUG_ASSERT(addon_length < UINT_MAX32); + res_length= addon_length; } else { @@ -93,11 +94,43 @@ void Sort_param::init_for_filesort(uint sortlen, TABLE *table, */ sort_length+= ref_length; } - rec_length= sort_length + (uint)addon_buf.length; + rec_length= sort_length + addon_length; max_rows= maxrows; } +void Sort_param::try_to_pack_addons(ulong max_length_for_sort_data) +{ + if (!using_addon_fields() || // no addons, or + using_packed_addons()) // already packed + return; + + if (!Addon_fields::can_pack_addon_fields(res_length)) + return; + + const uint sz= Addon_fields::size_of_length_field;; + if (rec_length + sz > max_length_for_sort_data) + return; + + // Heuristic: skip packing if potential savings are less than 10 bytes. + if (m_packable_length < (10 + sz)) + return; + + SORT_ADDON_FIELD *addonf= addon_fields->begin(); + for (;addonf != addon_fields->end(); ++addonf) + { + addonf->offset+= sz; + addonf->null_offset+= sz; + } + + addon_fields->set_using_packed_addons(true); + m_using_packed_addons= true; + + addon_length+= sz; + res_length+= sz; + rec_length+= sz; +} + /** Sort a table. Creates a set of pointers that can be used to read the rows @@ -134,7 +167,7 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, DBUG_ASSERT(thd->variables.sortbuff_size <= SIZE_T_MAX); size_t memory_available= (size_t)thd->variables.sortbuff_size; uint maxbuffer; - BUFFPEK *buffpek; + Merge_chunk *buffpek; ha_rows num_rows= HA_POS_ERROR; IO_CACHE tempfile, buffpek_pointers, *outfile; Sort_param param; @@ -164,13 +197,16 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, if (subselect && subselect->filesort_buffer.is_allocated()) { - /* Reuse cache from last call */ + // Reuse cache from last call sort->filesort_buffer= subselect->filesort_buffer; sort->buffpek= subselect->sortbuffer; subselect->filesort_buffer.reset(); subselect->sortbuffer.str=0; } + DBUG_ASSERT(sort->sorted_result_in_fsbuf == FALSE || + sort->record_pointers == NULL); + outfile= &sort->io_cache; my_b_clear(&tempfile); @@ -183,9 +219,8 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, &multi_byte_charset), table, max_rows, filesort->sort_positions); - sort->addon_buf= param.addon_buf; - sort->addon_field= param.addon_field; - sort->unpack= unpack_addon_fields; + sort->addon_fields= param.addon_fields; + if (multi_byte_charset && !(param.tmp_buffer= (char*) my_malloc(param.sort_length, MYF(MY_WME | MY_THREAD_SPECIFIC)))) @@ -208,7 +243,15 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, thd->query_plan_flags|= QPLAN_FILESORT_PRIORITY_QUEUE; status_var_increment(thd->status_var.filesort_pq_sorts_); tracker->incr_pq_used(); + param.using_pq= true; const size_t compare_length= param.sort_length; + /* + For PQ queries (with limit) we know exactly how many pointers/records + we have in the buffer, so to simplify things, we initialize + all pointers here. (We cannot pack fields anyways, so there is no + point in doing lazy initialization). + */ + sort->init_record_pointers(); if (pq.init(param.max_rows, true, // max_at_top NULL, // compare_function @@ -223,21 +266,23 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, DBUG_ASSERT(thd->is_error()); goto err; } - // For PQ queries (with limit) we initialize all pointers. - sort->init_record_pointers(); } else { DBUG_PRINT("info", ("filesort PQ is not applicable")); + param.try_to_pack_addons(thd->variables.max_length_for_sort_data); + param.using_pq= false; + size_t min_sort_memory= MY_MAX(MIN_SORT_MEMORY, param.sort_length*MERGEBUFF2); - set_if_bigger(min_sort_memory, sizeof(BUFFPEK*)*MERGEBUFF2); + set_if_bigger(min_sort_memory, sizeof(Merge_chunk*)*MERGEBUFF2); while (memory_available >= min_sort_memory) { ulonglong keys= memory_available / (param.rec_length + sizeof(char*)); param.max_keys_per_buffer= (uint) MY_MIN(num_rows, keys); - if (sort->alloc_sort_buffer(param.max_keys_per_buffer, param.rec_length)) + sort->alloc_sort_buffer(param.max_keys_per_buffer, param.rec_length); + if (sort->sort_buffer_size() > 0) break; size_t old_memory_available= memory_available; memory_available= memory_available/4*3; @@ -258,7 +303,9 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, goto err; param.sort_form= table; - param.end=(param.local_sortorder=filesort->sortorder)+s_length; + param.local_sortorder= + Bounds_checked_array<SORT_FIELD>(filesort->sortorder, s_length); + num_rows= find_all_keys(thd, &param, select, sort, &buffpek_pointers, @@ -287,12 +334,20 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, my_free(sort->buffpek.str); sort->buffpek.str= 0; } + + if (param.using_addon_fields()) + { + DBUG_ASSERT(sort->addon_fields); + if (!sort->addon_fields->allocate_addon_buf(param.addon_length)) + goto err; + } + if (!(sort->buffpek.str= (char *) read_buffpek_from_file(&buffpek_pointers, maxbuffer, (uchar*) sort->buffpek.str))) goto err; sort->buffpek.length= maxbuffer; - buffpek= (BUFFPEK *) sort->buffpek.str; + buffpek= (Merge_chunk *) sort->buffpek.str; close_cached_file(&buffpek_pointers); /* Open cached file if it isn't open */ if (! my_b_inited(outfile) && @@ -306,25 +361,25 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, Use also the space previously used by string pointers in sort_buffer for temporary key storage. */ - param.max_keys_per_buffer=((param.max_keys_per_buffer * - (param.rec_length + sizeof(char*))) / - param.rec_length - 1); + + param.max_keys_per_buffer= static_cast<uint>(sort->sort_buffer_size()) / + param.rec_length; set_if_bigger(param.max_keys_per_buffer, 1); maxbuffer--; // Offset from 0 - if (merge_many_buff(&param, - (uchar*) sort->get_sort_keys(), + + if (merge_many_buff(&param, sort->get_raw_buf(), buffpek,&maxbuffer, - &tempfile)) + &tempfile)) goto err; if (flush_io_cache(&tempfile) || reinit_io_cache(&tempfile,READ_CACHE,0L,0,0)) goto err; if (merge_index(&param, - (uchar*) sort->get_sort_keys(), + sort->get_raw_buf(), buffpek, maxbuffer, &tempfile, - outfile)) + outfile)) goto err; } @@ -339,7 +394,8 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, my_free(param.tmp_buffer); if (!subselect || !subselect->is_uncacheable()) { - sort->free_sort_buffer(); + if (!param.using_addon_fields()) + sort->free_sort_buffer(); my_free(sort->buffpek.str); } else @@ -347,7 +403,7 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, /* Remember sort buffers for next subquery call */ subselect->filesort_buffer= sort->filesort_buffer; subselect->sortbuffer= sort->buffpek; - sort->filesort_buffer.reset(); // Don't free this + sort->filesort_buffer.reset(); // Don't free this*/ } sort->buffpek.str= 0; @@ -361,7 +417,7 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, my_off_t save_pos=outfile->pos_in_file; /* For following reads */ if (reinit_io_cache(outfile,READ_CACHE,0L,0,0)) - error=1; + error=1; outfile->end_of_file=save_pos; } } @@ -490,10 +546,10 @@ uint Filesort::make_sortorder(THD *thd, JOIN *join, table_map first_table_bit) static uchar *read_buffpek_from_file(IO_CACHE *buffpek_pointers, uint count, uchar *buf) { - size_t length= sizeof(BUFFPEK)*count; + size_t length= sizeof(Merge_chunk)*count; uchar *tmp= buf; DBUG_ENTER("read_buffpek_from_file"); - if (count > UINT_MAX/sizeof(BUFFPEK)) + if (count > UINT_MAX/sizeof(Merge_chunk)) return 0; /* sizeof(BUFFPEK)*count will overflow */ if (!tmp) tmp= (uchar *)my_malloc(length, MYF(MY_WME | MY_THREAD_SPECIFIC)); @@ -702,7 +758,8 @@ static ha_rows find_all_keys(THD *thd, Sort_param *param, SQL_SELECT *select, handler *file; MY_BITMAP *save_read_set, *save_write_set; Item *sort_cond; - ha_rows retval; + ha_rows num_records= 0; + const bool packed_addon_fields= param->using_packed_addons(); DBUG_ENTER("find_all_keys"); DBUG_PRINT("info",("using: %s", (select ? select->quick ? "ranges" : "where": @@ -810,23 +867,27 @@ static ha_rows find_all_keys(THD *thd, Sort_param *param, SQL_SELECT *select, if (write_record) { - ++(*found_rows); if (pq) - { pq->push(ref_pos); - idx= pq->num_elements(); - } else { - if (idx == param->max_keys_per_buffer) + if (fs_info->isfull()) { if (write_keys(param, fs_info, idx, buffpek_pointers, tempfile)) goto err; - idx= 0; - indexpos++; + idx= 0; + indexpos++; } - make_sortkey(param, fs_info->get_record_buffer(idx++), ref_pos); + if (idx == 0) + fs_info->init_next_record_pointer(); + uchar *start_of_rec= fs_info->get_next_record_pointer(); + + const uint rec_sz= make_sortkey(param, start_of_rec, ref_pos); + if (packed_addon_fields && rec_sz != param->rec_length) + fs_info->adjust_next_record_pointer(rec_sz); + idx++; } + num_records++; } /* It does not make sense to read more keys in case of a fatal error */ @@ -862,11 +923,14 @@ static ha_rows find_all_keys(THD *thd, Sort_param *param, SQL_SELECT *select, if (indexpos && idx && write_keys(param, fs_info, idx, buffpek_pointers, tempfile)) DBUG_RETURN(HA_POS_ERROR); /* purecov: inspected */ - retval= (my_b_inited(tempfile) ? - (ha_rows) (my_b_tell(tempfile)/param->rec_length) : - idx); - DBUG_PRINT("info", ("find_all_keys return %llu", (ulonglong) retval)); - DBUG_RETURN(retval); + + (*found_rows)= num_records; + if (pq) + num_records= pq->num_elements(); + + + DBUG_PRINT("info", ("find_all_keys return %llu", (ulonglong) num_records)); + DBUG_RETURN(num_records); err: sort_form->column_bitmaps_set(save_read_set, save_write_set); @@ -901,36 +965,48 @@ write_keys(Sort_param *param, SORT_INFO *fs_info, uint count, IO_CACHE *buffpek_pointers, IO_CACHE *tempfile) { size_t rec_length; - uchar **end; - BUFFPEK buffpek; + Merge_chunk buffpek; DBUG_ENTER("write_keys"); rec_length= param->rec_length; - uchar **sort_keys= fs_info->get_sort_keys(); fs_info->sort_buffer(param, count); if (!my_b_inited(tempfile) && open_cached_file(tempfile, mysql_tmpdir, TEMP_PREFIX, DISK_BUFFER_SIZE, MYF(MY_WME))) - goto err; /* purecov: inspected */ + DBUG_RETURN(1); /* purecov: inspected */ /* check we won't have more buffpeks than we can possibly keep in memory */ - if (my_b_tell(buffpek_pointers) + sizeof(BUFFPEK) > (ulonglong)UINT_MAX) - goto err; + if (my_b_tell(buffpek_pointers) + sizeof(Merge_chunk) > (ulonglong)UINT_MAX) + DBUG_RETURN(1); + bzero(&buffpek, sizeof(buffpek)); - buffpek.file_pos= my_b_tell(tempfile); + buffpek.set_file_position(my_b_tell(tempfile)); if ((ha_rows) count > param->max_rows) count=(uint) param->max_rows; /* purecov: inspected */ - buffpek.count=(ha_rows) count; - for (end=sort_keys+count ; sort_keys != end ; sort_keys++) - if (my_b_write(tempfile, (uchar*) *sort_keys, (uint) rec_length)) - goto err; + buffpek.set_rowcount(static_cast<ha_rows>(count)); + + const bool packed_addon_fields= param->using_packed_addons(); + for (uint ix= 0; ix < count; ++ix) + { + uchar *record= fs_info->get_sorted_record(ix); + if (packed_addon_fields) + { + rec_length= param->sort_length + + Addon_fields::read_addon_length(record + param->sort_length); + } + else + rec_length= param->rec_length; + + if (my_b_write(tempfile, record, rec_length)) + DBUG_RETURN(1); /* purecov: inspected */ + } + if (my_b_write(buffpek_pointers, (uchar*) &buffpek, sizeof(buffpek))) - goto err; + DBUG_RETURN(1); + DBUG_RETURN(0); -err: - DBUG_RETURN(1); } /* write_keys */ @@ -1168,14 +1244,15 @@ Type_handler_real_result::make_sort_key(uchar *to, Item *item, /** Make a sort-key from record. */ -static void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos) +static uint make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos) { Field *field; SORT_FIELD *sort_field; uint length; + uchar *orig_to= to; - for (sort_field=param->local_sortorder ; - sort_field != param->end ; + for (sort_field=param->local_sortorder.begin() ; + sort_field != param->local_sortorder.end() ; sort_field++) { bool maybe_null=0; @@ -1202,15 +1279,15 @@ static void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos) length=sort_field->length; while (length--) { - *to = (uchar) (~ *to); - to++; + *to = (uchar) (~ *to); + to++; } } else to+= sort_field->length; } - if (param->addon_field) + if (param->using_addon_fields()) { /* Save field values appended to sorted fields. @@ -1218,41 +1295,44 @@ static void make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos) In this implementation we use fixed layout for field values - the same for all records. */ - SORT_ADDON_FIELD *addonf= param->addon_field; + SORT_ADDON_FIELD *addonf= param->addon_fields->begin(); uchar *nulls= to; + uchar *p_len= to; DBUG_ASSERT(addonf != 0); + const bool packed_addon_fields= param->addon_fields->using_packed_addons(); + uint32 res_len= addonf->offset; memset(nulls, 0, addonf->offset); to+= addonf->offset; - for ( ; (field= addonf->field) ; addonf++) + for ( ; addonf != param->addon_fields->end() ; addonf++) { + Field *field= addonf->field; if (addonf->null_bit && field->is_null()) { nulls[addonf->null_offset]|= addonf->null_bit; -#ifdef HAVE_valgrind - bzero(to, addonf->length); -#endif + if (!packed_addon_fields) + to+= addonf->length; } else { -#ifdef HAVE_valgrind uchar *end= field->pack(to, field->ptr); - uint length= (uint) ((to + addonf->length) - end); - DBUG_ASSERT((int) length >= 0); - if (length) - bzero(end, length); -#else - (void) field->pack(to, field->ptr); -#endif + int sz= static_cast<int>(end - to); + res_len += sz; + if (packed_addon_fields) + to+= sz; + else + to+= addonf->length; } - to+= addonf->length; } + if (packed_addon_fields) + Addon_fields::store_addon_length(p_len, res_len); } else { /* Save filepos last */ memcpy((uchar*) to, ref_pos, (size_t) param->ref_length); + to+= param->ref_length; } - return; + return static_cast<uint>(to - orig_to); } @@ -1265,8 +1345,8 @@ static void register_used_fields(Sort_param *param) SORT_FIELD *sort_field; TABLE *table=param->sort_form; - for (sort_field= param->local_sortorder ; - sort_field != param->end ; + for (sort_field= param->local_sortorder.begin() ; + sort_field != param->local_sortorder.end() ; sort_field++) { Field *field; @@ -1281,12 +1361,14 @@ static void register_used_fields(Sort_param *param) } } - if (param->addon_field) + if (param->using_addon_fields()) { - SORT_ADDON_FIELD *addonf= param->addon_field; - Field *field; - for ( ; (field= addonf->field) ; addonf++) + SORT_ADDON_FIELD *addonf= param->addon_fields->begin(); + for ( ; (addonf != param->addon_fields->end()) ; addonf++) + { + Field *field= addonf->field; field->register_field_in_read_map(); + } } else { @@ -1305,16 +1387,24 @@ static bool save_index(Sort_param *param, uint count, DBUG_ASSERT(table_sort->record_pointers == 0); table_sort->sort_buffer(param, count); + + if (param->using_addon_fields()) + { + table_sort->sorted_result_in_fsbuf= TRUE; + table_sort->set_sort_length(param->sort_length); + DBUG_RETURN(0); + } + res_length= param->res_length; offset= param->rec_length-res_length; if (!(to= table_sort->record_pointers= (uchar*) my_malloc(res_length*count, MYF(MY_WME | MY_THREAD_SPECIFIC)))) DBUG_RETURN(1); /* purecov: inspected */ - uchar **sort_keys= table_sort->get_sort_keys(); - for (uchar **end= sort_keys+count ; sort_keys != end ; sort_keys++) + for (uint ix= 0; ix < count; ++ix) { - memcpy(to, *sort_keys+offset, res_length); + uchar *record= table_sort->get_sorted_record(ix); + memcpy(to, record + offset, res_length); to+= res_length; } DBUG_RETURN(0); @@ -1385,8 +1475,9 @@ static bool check_if_pq_applicable(Sort_param *param, // The whole source set fits into memory. if (param->max_rows < num_rows/PQ_slowness ) { - DBUG_RETURN(filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, - param->rec_length) != NULL); + filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, + param->rec_length); + DBUG_RETURN(filesort_info->sort_buffer_size() != 0); } else { @@ -1398,12 +1489,13 @@ static bool check_if_pq_applicable(Sort_param *param, // Do we have space for LIMIT rows in memory? if (param->max_keys_per_buffer < num_available_keys) { - DBUG_RETURN(filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, - param->rec_length) != NULL); + filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, + param->rec_length); + DBUG_RETURN(filesort_info->sort_buffer_size() != 0); } // Try to strip off addon fields. - if (param->addon_field) + if (param->addon_fields) { const size_t row_length= param->sort_length + param->ref_length + sizeof(char*); @@ -1435,14 +1527,15 @@ static bool check_if_pq_applicable(Sort_param *param, if (sort_merge_cost < pq_cost) DBUG_RETURN(false); - if (filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, - param->sort_length + - param->ref_length)) + filesort_info->alloc_sort_buffer(param->max_keys_per_buffer, + param->sort_length + param->ref_length); + + if (filesort_info->sort_buffer_size() > 0) { /* Make attached data to be references instead of fields. */ - my_free(filesort_info->addon_field); - filesort_info->addon_field= NULL; - param->addon_field= NULL; + my_free(filesort_info->addon_fields); + filesort_info->addon_fields= NULL; + param->addon_fields= NULL; param->res_length= param->ref_length; param->sort_length+= param->ref_length; @@ -1458,12 +1551,12 @@ static bool check_if_pq_applicable(Sort_param *param, /** Merge buffers to make < MERGEBUFF2 buffers. */ -int merge_many_buff(Sort_param *param, uchar *sort_buffer, - BUFFPEK *buffpek, uint *maxbuffer, IO_CACHE *t_file) +int merge_many_buff(Sort_param *param, Sort_buffer sort_buffer, + Merge_chunk *buffpek, uint *maxbuffer, IO_CACHE *t_file) { uint i; IO_CACHE t_file2,*from_file,*to_file,*temp; - BUFFPEK *lastbuff; + Merge_chunk *lastbuff; DBUG_ENTER("merge_many_buff"); if (*maxbuffer < MERGEBUFF2) @@ -1483,11 +1576,11 @@ int merge_many_buff(Sort_param *param, uchar *sort_buffer, lastbuff=buffpek; for (i=0 ; i <= *maxbuffer-MERGEBUFF*3/2 ; i+=MERGEBUFF) { - if (merge_buffers(param,from_file,to_file,sort_buffer,lastbuff++, + if (merge_buffers(param,from_file,to_file,sort_buffer, lastbuff++, buffpek+i,buffpek+i+MERGEBUFF-1,0)) goto cleanup; } - if (merge_buffers(param,from_file,to_file,sort_buffer,lastbuff++, + if (merge_buffers(param,from_file,to_file,sort_buffer, lastbuff++, buffpek+i,buffpek+ *maxbuffer,0)) break; /* purecov: inspected */ if (flush_io_cache(to_file)) @@ -1513,24 +1606,68 @@ int merge_many_buff(Sort_param *param, uchar *sort_buffer, (ulong)-1 if something goes wrong */ -ulong read_to_buffer(IO_CACHE *fromfile, BUFFPEK *buffpek, - uint rec_length) +ulong read_to_buffer(IO_CACHE *fromfile, Merge_chunk *buffpek, + Sort_param *param) { - ulong count; - ulong length= 0; + ha_rows count; + uint rec_length= param->rec_length; - if ((count= (ulong) MY_MIN((ha_rows) buffpek->max_keys,buffpek->count))) + if ((count= MY_MIN(buffpek->max_keys(),buffpek->rowcount()))) { - length= rec_length*count; - if (unlikely(my_b_pread(fromfile, (uchar*) buffpek->base, length, - buffpek->file_pos))) + size_t bytes_to_read; + if (param->using_packed_addons()) + { + count= buffpek->rowcount(); + bytes_to_read= MY_MIN(buffpek->buffer_size(), + static_cast<size_t>(fromfile->end_of_file - + buffpek->file_position())); + } + else + bytes_to_read= rec_length * static_cast<size_t>(count); + + if (unlikely(my_b_pread(fromfile, buffpek->buffer_start(), + bytes_to_read, buffpek->file_position()))) return ((ulong) -1); - buffpek->key=buffpek->base; - buffpek->file_pos+= length; /* New filepos */ - buffpek->count-= count; - buffpek->mem_count= count; + + size_t num_bytes_read; + if (param->using_packed_addons()) + { + /* + The last record read is most likely not complete here. + We need to loop through all the records, reading the length fields, + and then "chop off" the final incomplete record. + */ + uchar *record= buffpek->buffer_start(); + uint ix= 0; + for (; ix < count; ++ix) + { + if (record + param->sort_length + Addon_fields::size_of_length_field > + buffpek->buffer_end()) + break; // Incomplete record. + uchar *plen= record + param->sort_length; + uint res_length= Addon_fields::read_addon_length(plen); + if (plen + res_length > buffpek->buffer_end()) + break; // Incomplete record. + DBUG_ASSERT(res_length > 0); + record+= param->sort_length; + record+= res_length; + } + DBUG_ASSERT(ix > 0); + count= ix; + num_bytes_read= record - buffpek->buffer_start(); + DBUG_PRINT("info", ("read %llu bytes of complete records", + static_cast<ulonglong>(bytes_to_read))); + } + else + num_bytes_read= bytes_to_read; + + buffpek->init_current_key(); + buffpek->advance_file_position(num_bytes_read); /* New filepos */ + buffpek->decrement_rowcount(count); + buffpek->set_mem_count(count); + return (ulong) num_bytes_read; } - return (length); + return 0; } /* read_to_buffer */ @@ -1545,25 +1682,15 @@ ulong read_to_buffer(IO_CACHE *fromfile, BUFFPEK *buffpek, @param[in] key_length key length */ -void reuse_freed_buff(QUEUE *queue, BUFFPEK *reuse, uint key_length) +void reuse_freed_buff(QUEUE *queue, Merge_chunk *reuse, uint key_length) { - uchar *reuse_end= reuse->base + reuse->max_keys * key_length; for (uint i= queue_first_element(queue); i <= queue_last_element(queue); i++) { - BUFFPEK *bp= (BUFFPEK *) queue_element(queue, i); - if (bp->base + bp->max_keys * key_length == reuse->base) - { - bp->max_keys+= reuse->max_keys; + Merge_chunk *bp= (Merge_chunk *) queue_element(queue, i); + if (reuse->merge_freed_buff(bp)) return; - } - else if (bp->base == reuse_end) - { - bp->base= reuse->base; - bp->max_keys+= reuse->max_keys; - return; - } } DBUG_ASSERT(0); } @@ -1588,8 +1715,8 @@ void reuse_freed_buff(QUEUE *queue, BUFFPEK *reuse, uint key_length) */ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, - IO_CACHE *to_file, uchar *sort_buffer, - BUFFPEK *lastbuff, BUFFPEK *Fb, BUFFPEK *Tb, + IO_CACHE *to_file, Sort_buffer sort_buffer, + Merge_chunk *lastbuff, Merge_chunk *Fb, Merge_chunk *Tb, int flag) { bool error= 0; @@ -1599,7 +1726,7 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, ha_rows max_rows,org_max_rows; my_off_t to_start_filepos; uchar *strpos; - BUFFPEK *buffpek; + Merge_chunk *buffpek; QUEUE queue; qsort2_cmp cmp; void *first_cmp_arg; @@ -1625,7 +1752,7 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, uint wr_offset= flag ? offset : 0; maxcount= (ulong) (param->max_keys_per_buffer/((uint) (Tb-Fb) +1)); to_start_filepos= my_b_tell(to_file); - strpos= sort_buffer; + strpos= sort_buffer.array(); org_max_rows=max_rows= param->max_rows; set_if_bigger(maxcount, 1); @@ -1640,19 +1767,23 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, cmp= get_ptr_compare(sort_length); first_cmp_arg= (void*) &sort_length; } - if (unlikely(init_queue(&queue, (uint) (Tb-Fb)+1, offsetof(BUFFPEK,key), 0, + if (unlikely(init_queue(&queue, (uint) (Tb-Fb)+1, + offsetof(Merge_chunk,m_current_key), 0, (queue_compare) cmp, first_cmp_arg, 0, 0))) DBUG_RETURN(1); /* purecov: inspected */ for (buffpek= Fb ; buffpek <= Tb ; buffpek++) { - buffpek->base= strpos; - buffpek->max_keys= maxcount; - bytes_read= read_to_buffer(from_file, buffpek, rec_length); + buffpek->set_buffer(strpos, + strpos + (sort_buffer.size()/((uint) (Tb-Fb) +1))); + + buffpek->set_max_keys(maxcount); + bytes_read= read_to_buffer(from_file, buffpek, param); if (unlikely(bytes_read == (ulong) -1)) goto err; /* purecov: inspected */ - strpos+= bytes_read; - buffpek->max_keys= buffpek->mem_count; // If less data in buffers than expected + buffpek->set_buffer_end(strpos); + // If less data in buffers than expected + buffpek->set_max_keys(buffpek->mem_count()); queue_insert(&queue, (uchar*) buffpek); } @@ -1663,16 +1794,17 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, Copy the first argument to unique_buff for unique removal. Store it also in 'to_file'. */ - buffpek= (BUFFPEK*) queue_top(&queue); - memcpy(unique_buff, buffpek->key, rec_length); + buffpek= (Merge_chunk*) queue_top(&queue); + memcpy(unique_buff, buffpek->current_key(), rec_length); if (min_dupl_count) memcpy(&dupl_count, unique_buff+dupl_count_ofs, sizeof(dupl_count)); - buffpek->key+= rec_length; - if (! --buffpek->mem_count) + buffpek->advance_current_key(rec_length); + buffpek->decrement_mem_count(); + if (buffpek->mem_count() == 0) { if (unlikely(!(bytes_read= read_to_buffer(from_file, buffpek, - rec_length)))) + param)))) { (void) queue_remove_top(&queue); reuse_freed_buff(&queue, buffpek, rec_length); @@ -1692,61 +1824,68 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, for (;;) { - buffpek= (BUFFPEK*) queue_top(&queue); - src= buffpek->key; + buffpek= (Merge_chunk*) queue_top(&queue); + src= buffpek->current_key(); if (cmp) // Remove duplicates { - if (!(*cmp)(first_cmp_arg, &unique_buff, - (uchar**) &buffpek->key)) - { + uchar *current_key= buffpek->current_key(); + if (!(*cmp)(first_cmp_arg, &unique_buff, &current_key)) + { if (min_dupl_count) - { + { element_count cnt; - memcpy(&cnt, (uchar *) buffpek->key+dupl_count_ofs, sizeof(cnt)); + memcpy(&cnt, buffpek->current_key() + dupl_count_ofs, sizeof(cnt)); dupl_count+= cnt; } goto skip_duplicate; } if (min_dupl_count) - { + { memcpy(unique_buff+dupl_count_ofs, &dupl_count, sizeof(dupl_count)); } - src= unique_buff; - } - - /* - Do not write into the output file if this is the final merge called - for a Unique object used for intersection and dupl_count is less - than min_dupl_count. - If the Unique object is used to intersect N sets of unique elements - then for any element: - dupl_count >= N <=> the element is occurred in each of these N sets. - */ - if (!check_dupl_count || dupl_count >= min_dupl_count) - { - if (my_b_write(to_file, src+wr_offset, wr_len)) - goto err; /* purecov: inspected */ + src= unique_buff; } - if (cmp) - { - memcpy(unique_buff, (uchar*) buffpek->key, rec_length); - if (min_dupl_count) - memcpy(&dupl_count, unique_buff+dupl_count_ofs, - sizeof(dupl_count)); - } - if (!--max_rows) + { - /* Nothing more to do */ - goto end; /* purecov: inspected */ - } + param->get_rec_and_res_len(buffpek->current_key(), + &rec_length, &res_length); + const uint bytes_to_write= (flag == 0) ? rec_length : res_length; + /* + Do not write into the output file if this is the final merge called + for a Unique object used for intersection and dupl_count is less + than min_dupl_count. + If the Unique object is used to intersect N sets of unique elements + then for any element: + dupl_count >= N <=> the element is occurred in each of these N sets. + */ + if (!check_dupl_count || dupl_count >= min_dupl_count) + { + if (my_b_write(to_file, src + wr_offset, bytes_to_write)) + goto err; /* purecov: inspected */ + } + if (cmp) + { + memcpy(unique_buff, buffpek->current_key(), rec_length); + if (min_dupl_count) + memcpy(&dupl_count, unique_buff+dupl_count_ofs, + sizeof(dupl_count)); + } + if (!--max_rows) + { + /* Nothing more to do */ + goto end; /* purecov: inspected */ + } + } skip_duplicate: - buffpek->key+= rec_length; - if (! --buffpek->mem_count) + buffpek->advance_current_key(rec_length); + buffpek->decrement_mem_count(); + + if (buffpek->mem_count() == 0) { if (unlikely(!(bytes_read= read_to_buffer(from_file, buffpek, - rec_length)))) + param)))) { (void) queue_remove_top(&queue); reuse_freed_buff(&queue, buffpek, rec_length); @@ -1758,9 +1897,10 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, queue_replace_top(&queue); /* Top element has been replaced */ } } - buffpek= (BUFFPEK*) queue_top(&queue); - buffpek->base= (uchar*) sort_buffer; - buffpek->max_keys= param->max_keys_per_buffer; + buffpek= (Merge_chunk*) queue_top(&queue); + buffpek->set_buffer(sort_buffer.array(), + sort_buffer.array() + sort_buffer.size()); + buffpek->set_max_keys(param->max_keys_per_buffer); /* As we know all entries in the buffer are unique, we only have to @@ -1768,16 +1908,17 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, */ if (cmp) { - if (!(*cmp)(first_cmp_arg, &unique_buff, (uchar**) &buffpek->key)) + uchar *current_key= buffpek->current_key(); + if (!(*cmp)(first_cmp_arg, &unique_buff, &current_key)) { if (min_dupl_count) { element_count cnt; - memcpy(&cnt, (uchar *) buffpek->key+dupl_count_ofs, sizeof(cnt)); + memcpy(&cnt, buffpek->current_key() + dupl_count_ofs, sizeof(cnt)); dupl_count+= cnt; } - buffpek->key+= rec_length; - --buffpek->mem_count; + buffpek->advance_current_key(rec_length); + buffpek->decrement_mem_count(); } if (min_dupl_count) @@ -1796,45 +1937,40 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, do { - if ((ha_rows) buffpek->mem_count > max_rows) + if (buffpek->mem_count() > max_rows) { /* Don't write too many records */ - buffpek->mem_count= (uint) max_rows; - buffpek->count= 0; /* Don't read more */ + buffpek->set_mem_count(max_rows); + buffpek->set_rowcount(0); /* Don't read more */ } - max_rows-= buffpek->mem_count; - if (flag == 0) + max_rows-= buffpek->mem_count(); + for (uint ix= 0; ix < buffpek->mem_count(); ++ix) { - if (my_b_write(to_file, (uchar*) buffpek->key, - (size_t)(rec_length*buffpek->mem_count))) - goto err; /* purecov: inspected */ - } - else - { - uchar *end; - src= buffpek->key+offset; - for (end= src+buffpek->mem_count*rec_length ; - src != end ; - src+= rec_length) + param->get_rec_and_res_len(buffpek->current_key(), + &rec_length, &res_length); + const uint bytes_to_write= (flag == 0) ? rec_length : res_length; + if (check_dupl_count) { - if (check_dupl_count) - { - memcpy((uchar *) &dupl_count, src+dupl_count_ofs, sizeof(dupl_count)); - if (dupl_count < min_dupl_count) - continue; - } - if (my_b_write(to_file, src, wr_len)) - goto err; + memcpy((uchar *) &dupl_count, + buffpek->current_key() + offset + dupl_count_ofs, + sizeof(dupl_count)); + if (dupl_count < min_dupl_count) + continue; } + if (my_b_write(to_file, buffpek->current_key() + wr_offset, + bytes_to_write)) + goto err; + buffpek->advance_current_key(rec_length); } } while (likely(!(error= (bytes_read= read_to_buffer(from_file, buffpek, - rec_length)) == (ulong) -1)) && + param)) == (ulong) -1)) && bytes_read != 0); end: - lastbuff->count= MY_MIN(org_max_rows-max_rows, param->max_rows); - lastbuff->file_pos= to_start_filepos; + lastbuff->set_rowcount(MY_MIN(org_max_rows-max_rows, param->max_rows)); + lastbuff->set_file_position(to_start_filepos); + cleanup: delete_queue(&queue); DBUG_RETURN(error); @@ -1848,13 +1984,13 @@ bool merge_buffers(Sort_param *param, IO_CACHE *from_file, /* Do a merge to output-file (save only positions) */ -int merge_index(Sort_param *param, uchar *sort_buffer, - BUFFPEK *buffpek, uint maxbuffer, - IO_CACHE *tempfile, IO_CACHE *outfile) +int merge_index(Sort_param *param, Sort_buffer sort_buffer, + Merge_chunk *buffpek, uint maxbuffer, + IO_CACHE *tempfile, IO_CACHE *outfile) { DBUG_ENTER("merge_index"); - if (merge_buffers(param,tempfile,outfile,sort_buffer,buffpek,buffpek, - buffpek+maxbuffer,1)) + if (merge_buffers(param, tempfile, outfile, sort_buffer, buffpek, buffpek, + buffpek + maxbuffer, 1)) DBUG_RETURN(1); /* purecov: inspected */ DBUG_RETURN(0); } /* merge_index */ @@ -1977,7 +2113,7 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length, sortorder->length= (uint)cs->coll->strnxfrmlen(cs, sortorder->length); } if (sortorder->field->maybe_null()) - length++; // Place for NULL marker + length++; // Place for NULL marker } else { @@ -1988,21 +2124,40 @@ sortlength(THD *thd, SORT_FIELD *sortorder, uint s_length, *multi_byte_charset= true; } if (sortorder->item->maybe_null) - length++; // Place for NULL marker + length++; // Place for NULL marker } set_if_smaller(sortorder->length, thd->variables.max_sort_length); length+=sortorder->length; } - sortorder->field= (Field*) 0; // end marker + sortorder->field= NULL; // end marker DBUG_PRINT("info",("sort_length: %d",length)); return length; } + +/* + Check whether addon fields can be used or not. + + @param table Table structure + @param sortlength Length of sort key + @param length [OUT] Max length of addon fields + @param fields [OUT] Number of addon fields + @param null_fields [OUT] Number of nullable addon fields + @param packable_length [OUT] Max length of addon fields that can be + packed + + @retval + TRUE Addon fields can be used + FALSE Otherwise +*/ + bool filesort_use_addons(TABLE *table, uint sortlength, - uint *length, uint *fields, uint *null_fields) + uint *length, uint *fields, uint *null_fields, + uint *packable_length) { Field **pfield, *field; - *length= *fields= *null_fields= 0; + *length= *fields= *null_fields= *packable_length= 0; + uint field_length=0; for (pfield= table->field; (field= *pfield) ; pfield++) { @@ -2010,7 +2165,12 @@ bool filesort_use_addons(TABLE *table, uint sortlength, continue; if (field->flags & BLOB_FLAG) return false; - (*length)+= field->max_packed_col_length(field->pack_length()); + field_length= field->max_packed_col_length(field->pack_length()); + (*length)+= field_length; + + if (field->maybe_null() || field->is_packable()) + (*packable_length)+= field_length; + if (field->maybe_null()) (*null_fields)++; (*fields)++; @@ -2035,11 +2195,11 @@ bool filesort_use_addons(TABLE *table, uint sortlength, layouts for the values of the non-sorted fields in the buffer and fills them. - @param thd Current thread - @param ptabfield Array of references to the table fields - @param sortlength Total length of sorted fields - @param [out] addon_buf Buffer to us for appended fields - + @param table Table structure + @param sortlength Total length of sorted fields + @param addon_length [OUT] Length of addon fields + @param m_packable_length [OUT] Length of the addon fields that can be + packed @note The null bits for the appended values are supposed to be put together and stored the buffer just ahead of the value of the first field. @@ -2050,13 +2210,13 @@ bool filesort_use_addons(TABLE *table, uint sortlength, NULL if we do not store field values with sort data. */ -static SORT_ADDON_FIELD * -get_addon_fields(TABLE *table, uint sortlength, LEX_STRING *addon_buf) +static Addon_fields* +get_addon_fields(TABLE *table, uint sortlength, + uint *addon_length, uint *m_packable_length) { Field **pfield; Field *field; - SORT_ADDON_FIELD *addonf; - uint length, fields, null_fields; + uint length, fields, null_fields, packable_length; MY_BITMAP *read_set= table->read_set; DBUG_ENTER("get_addon_fields"); @@ -2070,23 +2230,34 @@ get_addon_fields(TABLE *table, uint sortlength, LEX_STRING *addon_buf) the values directly from sorted fields. But beware the case when item->cmp_type() != item->result_type() */ - addon_buf->str= 0; - addon_buf->length= 0; // see remove_const() for HA_SLOW_RND_POS explanation if (table->file->ha_table_flags() & HA_SLOW_RND_POS) sortlength= 0; - if (!filesort_use_addons(table, sortlength, &length, &fields, &null_fields) || - !my_multi_malloc(MYF(MY_WME | MY_THREAD_SPECIFIC), &addonf, - sizeof(SORT_ADDON_FIELD) * (fields+1), - &addon_buf->str, length, NullS)) + void *raw_mem_addon_field, *raw_mem; + if (!filesort_use_addons(table, sortlength, &length, &fields, &null_fields, + &packable_length) || + !(my_multi_malloc(MYF(MY_WME | MY_THREAD_SPECIFIC), + &raw_mem, sizeof(Addon_fields), + &raw_mem_addon_field, + sizeof(SORT_ADDON_FIELD) * fields, + NullS))) DBUG_RETURN(0); - addon_buf->length= length; + Addon_fields_array + addon_array(static_cast<SORT_ADDON_FIELD*>(raw_mem_addon_field), fields); + Addon_fields *addon_fields= new (raw_mem) Addon_fields(addon_array); + + DBUG_ASSERT(addon_fields); + + (*addon_length)= length; + (*m_packable_length)= packable_length; + length= (null_fields+7)/8; null_fields= 0; + SORT_ADDON_FIELD* addonf= addon_fields->begin(); for (pfield= table->field; (field= *pfield) ; pfield++) { if (!bitmap_is_set(read_set, field->field_index)) @@ -2108,10 +2279,9 @@ get_addon_fields(TABLE *table, uint sortlength, LEX_STRING *addon_buf) length+= addonf->length; addonf++; } - addonf->field= 0; // Put end marker DBUG_PRINT("info",("addon_length: %d",length)); - DBUG_RETURN(addonf-fields); + DBUG_RETURN(addon_fields); } @@ -2130,24 +2300,7 @@ get_addon_fields(TABLE *table, uint sortlength, LEX_STRING *addon_buf) void. */ -static void -unpack_addon_fields(struct st_sort_addon_field *addon_field, uchar *buff, - uchar *buff_end) -{ - Field *field; - SORT_ADDON_FIELD *addonf= addon_field; - for ( ; (field= addonf->field) ; addonf++) - { - if (addonf->null_bit && (addonf->null_bit & buff[addonf->null_offset])) - { - field->set_null(); - continue; - } - field->set_notnull(); - field->unpack(field->ptr, buff + addonf->offset, buff_end, 0); - } -} /* ** functions to change a double or float to a sortable string @@ -2197,6 +2350,17 @@ void change_double_for_sort(double nr,uchar *to) } } +bool SORT_INFO::using_packed_addons() +{ + return addon_fields != NULL && addon_fields->using_packed_addons(); +} + +void SORT_INFO::free_addon_buff() +{ + if (addon_fields) + addon_fields->free_addon_buff(); +} + /** Free SORT_INFO */ diff --git a/sql/filesort.h b/sql/filesort.h index 5f79a5095cc..5102ee2326f 100644 --- a/sql/filesort.h +++ b/sql/filesort.h @@ -27,7 +27,7 @@ class Filesort_tracker; struct SORT_FIELD; typedef struct st_order ORDER; class JOIN; - +class Addon_fields; /** Sorting related info. @@ -87,7 +87,8 @@ class SORT_INFO public: SORT_INFO() - :addon_field(0), record_pointers(0) + :addon_fields(NULL), record_pointers(0), + sorted_result_in_fsbuf(FALSE) { buffpek.str= 0; my_b_clear(&io_cache); @@ -98,9 +99,11 @@ class SORT_INFO void free_data() { close_cached_file(&io_cache); + free_addon_buff(); my_free(record_pointers); my_free(buffpek.str); - my_free(addon_field); + my_free(addon_fields); + free_sort_buffer(); } void reset() @@ -108,17 +111,26 @@ class SORT_INFO free_data(); record_pointers= 0; buffpek.str= 0; - addon_field= 0; + addon_fields= 0; + sorted_result_in_fsbuf= false; } + void free_addon_buff(); IO_CACHE io_cache; /* If sorted through filesort */ LEX_STRING buffpek; /* Buffer for buffpek structures */ - LEX_STRING addon_buf; /* Pointer to a buffer if sorted with fields */ - struct st_sort_addon_field *addon_field; /* Pointer to the fields info */ - /* To unpack back */ - void (*unpack)(struct st_sort_addon_field *, uchar *, uchar *); + Addon_fields *addon_fields; /* Addon field descriptors */ uchar *record_pointers; /* If sorted in memory */ + + /** + If the entire result of filesort fits in memory, we skip the merge phase. + We may leave the result in filesort_buffer + (indicated by sorted_result_in_fsbuf), or we may strip away + the sort keys, and copy the sorted result into a new buffer. + @see save_index() + */ + bool sorted_result_in_fsbuf; + /* How many rows in final result. Also how many rows in record_pointers, if used @@ -131,27 +143,65 @@ class SORT_INFO void sort_buffer(Sort_param *param, uint count) { filesort_buffer.sort_buffer(param, count); } - /** - Accessors for Filesort_buffer (which @c). - */ - uchar *get_record_buffer(uint idx) - { return filesort_buffer.get_record_buffer(idx); } - uchar **get_sort_keys() { return filesort_buffer.get_sort_keys(); } - uchar **alloc_sort_buffer(uint num_records, uint record_length) + uchar *get_sorted_record(uint ix) + { return filesort_buffer.get_sorted_record(ix); } + + uchar *alloc_sort_buffer(uint num_records, uint record_length) { return filesort_buffer.alloc_sort_buffer(num_records, record_length); } void free_sort_buffer() { filesort_buffer.free_sort_buffer(); } + bool isfull() const + { return filesort_buffer.isfull(); } void init_record_pointers() { filesort_buffer.init_record_pointers(); } + void init_next_record_pointer() + { filesort_buffer.init_next_record_pointer(); } + uchar *get_next_record_pointer() + { return filesort_buffer.get_next_record_pointer(); } + void adjust_next_record_pointer(uint val) + { filesort_buffer.adjust_next_record_pointer(val); } + + Bounds_checked_array<uchar> get_raw_buf() + { return filesort_buffer.get_raw_buf(); } size_t sort_buffer_size() const { return filesort_buffer.sort_buffer_size(); } + bool is_allocated() const + { return filesort_buffer.is_allocated(); } + void set_sort_length(uint val) + { filesort_buffer.set_sort_length(val); } + uint get_sort_length() const + { return filesort_buffer.get_sort_length(); } + + bool has_filesort_result_in_memory() const + { + return record_pointers || sorted_result_in_fsbuf; + } + + /// Are we using "addon fields"? + bool using_addon_fields() const + { + return addon_fields != NULL; + } + + /// Are we using "packed addon fields"? + bool using_packed_addons(); + + /** + Copies (unpacks) values appended to sorted fields from a buffer back to + their regular positions specified by the Field::ptr pointers. + @param buff Buffer which to unpack the value from + */ + template<bool Packed_addon_fields> + inline void unpack_addon_fields(uchar *buff); + + friend SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, Filesort_tracker* tracker, JOIN *join, table_map first_table_bit); @@ -162,7 +212,8 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, table_map first_table_bit=0); bool filesort_use_addons(TABLE *table, uint sortlength, - uint *length, uint *fields, uint *null_fields); + uint *length, uint *fields, uint *null_fields, + uint *m_packable_length); void change_double_for_sort(double nr,uchar *to); diff --git a/sql/filesort_utils.cc b/sql/filesort_utils.cc index 703db84495f..06e3f477993 100644 --- a/sql/filesort_utils.cc +++ b/sql/filesort_utils.cc @@ -96,82 +96,92 @@ double get_merge_many_buffs_cost_fast(ha_rows num_rows, # Pointer to allocated buffer */ -uchar **Filesort_buffer::alloc_sort_buffer(uint num_records, - uint record_length) +uchar *Filesort_buffer::alloc_sort_buffer(uint num_records, + uint record_length) { size_t buff_size; - uchar **sort_keys, **start_of_data; DBUG_ENTER("alloc_sort_buffer"); DBUG_EXECUTE_IF("alloc_sort_buffer_fail", DBUG_SET("+d,simulate_out_of_memory");); - buff_size= ((size_t)num_records) * (record_length + sizeof(uchar*)); - set_if_bigger(buff_size, record_length * MERGEBUFF2); + buff_size= ALIGN_SIZE(num_records * (record_length + sizeof(uchar*))); - if (!m_idx_array.is_null()) + /* + The minimum memory required should be each merge buffer can hold atmost + one key. + TODO varun: move this to the place where min_sort_memory is used. + */ + set_if_bigger(buff_size, (record_length +sizeof(uchar*)) * MERGEBUFF2); + + if (m_rawmem) { /* Reuse old buffer if exists and is large enough Note that we don't make the buffer smaller, as we want to be prepared for next subquery iteration. */ - - sort_keys= m_idx_array.array(); - if (buff_size > allocated_size) + if (buff_size > m_size_in_bytes) { /* Better to free and alloc than realloc as we don't have to remember the old values */ - my_free(sort_keys); - if (!(sort_keys= (uchar**) my_malloc(buff_size, - MYF(MY_THREAD_SPECIFIC)))) + my_free(m_rawmem); + if (!(m_rawmem= (uchar*) my_malloc(buff_size, MYF(MY_THREAD_SPECIFIC)))) { - reset(); + m_size_in_bytes= 0; DBUG_RETURN(0); } - allocated_size= buff_size; } } else { - if (!(sort_keys= (uchar**) my_malloc(buff_size, MYF(MY_THREAD_SPECIFIC)))) + if (!(m_rawmem= (uchar*) my_malloc(buff_size, MYF(MY_THREAD_SPECIFIC)))) + { + m_size_in_bytes= 0; DBUG_RETURN(0); - allocated_size= buff_size; + } + } - m_idx_array= Idx_array(sort_keys, num_records); + m_size_in_bytes= buff_size; + m_record_pointers= reinterpret_cast<uchar**>(m_rawmem) + + ((m_size_in_bytes / sizeof(uchar*)) - 1); + m_num_records= num_records; m_record_length= record_length; - start_of_data= m_idx_array.array() + m_idx_array.size(); - m_start_of_data= reinterpret_cast<uchar*>(start_of_data); - - DBUG_RETURN(m_idx_array.array()); + m_idx= 0; + DBUG_RETURN(m_rawmem); } void Filesort_buffer::free_sort_buffer() { - my_free(m_idx_array.array()); - m_idx_array.reset(); - m_start_of_data= NULL; + my_free(m_rawmem); + *this= Filesort_buffer(); } void Filesort_buffer::sort_buffer(const Sort_param *param, uint count) { size_t size= param->sort_length; + m_sort_keys= get_sort_keys(); + if (count <= 1 || size == 0) return; - uchar **keys= get_sort_keys(); + + // dont reverse for PQ, it is already done + if (!param->using_pq) + reverse_record_pointers(); + uchar **buffer= NULL; if (radixsort_is_appliccable(count, param->sort_length) && (buffer= (uchar**) my_malloc(count*sizeof(char*), MYF(MY_THREAD_SPECIFIC)))) { - radixsort_for_str_ptr(keys, count, param->sort_length, buffer); + radixsort_for_str_ptr(m_sort_keys, count, param->sort_length, buffer); my_free(buffer); return; } - my_qsort2(keys, count, sizeof(uchar*), get_ptr_compare(size), &size); + my_qsort2(m_sort_keys, count, sizeof(uchar*), get_ptr_compare(size), &size); } diff --git a/sql/filesort_utils.h b/sql/filesort_utils.h index 1ab1ba2daa8..e8b93940abf 100644 --- a/sql/filesort_utils.h +++ b/sql/filesort_utils.h @@ -46,68 +46,194 @@ double get_merge_many_buffs_cost_fast(ha_rows num_rows, /** A wrapper class around the buffer used by filesort(). - The buffer is a contiguous chunk of memory, - where the first part is <num_records> pointers to the actual data. + The sort buffer is a contiguous chunk of memory, + containing both records to be sorted, and pointers to said records: + + <start of buffer | still unused | end of buffer> + |rec 0|record 1 |rec 2| ............ |ptr to rec2|ptr to rec1|ptr to rec0| + + Records will be inserted "left-to-right". Records are not necessarily + fixed-size, they can be packed and stored without any "gaps". + + Record pointers will be inserted "right-to-left", as a side-effect + of inserting the actual records. We wrap the buffer in order to be able to do lazy initialization of the pointers: the buffer is often much larger than what we actually need. + With this allocation scheme, and lazy initialization of the pointers, + we are able to pack variable-sized records in the buffer, + and thus possibly have space for more records than we initially estimated. + The buffer must be kept available for multiple executions of the same sort operation, so we have explicit allocate and free functions, rather than doing alloc/free in CTOR/DTOR. */ + class Filesort_buffer { public: - Filesort_buffer() - : m_idx_array(), m_start_of_data(NULL), allocated_size(0) + Filesort_buffer() : + m_next_rec_ptr(NULL), m_rawmem(NULL), m_record_pointers(NULL), + m_sort_keys(NULL), + m_num_records(0), m_record_length(0), + m_sort_length(0), + m_size_in_bytes(0), m_idx(0) {} - - ~Filesort_buffer() + + /** Sort me... */ + void sort_buffer(const Sort_param *param, uint count); + + /** + Reverses the record pointer array, to avoid recording new results for + non-deterministic mtr tests. + */ + void reverse_record_pointers() { - my_free(m_idx_array.array()); + if (m_idx < 2) // There is nothing to swap. + return; + uchar **keys= get_sort_keys(); + const longlong count= m_idx - 1; + for (longlong ix= 0; ix <= count/2; ++ix) + { + uchar *tmp= keys[count - ix]; + keys[count - ix] = keys[ix]; + keys[ix]= tmp; + } } - bool is_allocated() + /** + Initializes all the record pointers. + */ + void init_record_pointers() { - return m_idx_array.array() != 0; + init_next_record_pointer(); + while (m_idx < m_num_records) + (void) get_next_record_pointer(); + reverse_record_pointers(); } - void reset() + + /** + Prepares the buffer for the next batch of records to process. + */ + void init_next_record_pointer() { - m_idx_array.reset(); + m_idx= 0; + m_next_rec_ptr= m_rawmem; + m_sort_keys= NULL; } - /** Sort me... */ - void sort_buffer(const Sort_param *param, uint count); + /** + @returns the number of bytes currently in use for data. + */ + size_t space_used_for_data() const + { + return m_next_rec_ptr ? m_next_rec_ptr - m_rawmem : 0; + } - /// Initializes a record pointer. - uchar *get_record_buffer(uint idx) + /** + @returns the number of bytes left in the buffer. + */ + size_t spaceleft() const { - m_idx_array[idx]= m_start_of_data + (idx * m_record_length); - return m_idx_array[idx]; + DBUG_ASSERT(m_next_rec_ptr >= m_rawmem); + const size_t spaceused= + (m_next_rec_ptr - m_rawmem) + + (static_cast<size_t>(m_idx) * sizeof(uchar*)); + return m_size_in_bytes - spaceused; } - /// Initializes all the record pointers. - void init_record_pointers() + /** + Is the buffer full? + */ + bool isfull() const + { + if (m_idx < m_num_records) + return false; + return spaceleft() < (m_record_length + sizeof(uchar*)); + } + + /** + Where should the next record be stored? + */ + uchar *get_next_record_pointer() + { + uchar *retval= m_next_rec_ptr; + // Save the return value in the record pointer array. + m_record_pointers[-m_idx]= m_next_rec_ptr; + // Prepare for the subsequent request. + m_idx++; + m_next_rec_ptr+= m_record_length; + return retval; + } + + /** + Adjusts for actual record length. get_next_record_pointer() above was + pessimistic, and assumed that the record could not be packed. + */ + void adjust_next_record_pointer(uint val) { - for (uint ix= 0; ix < m_idx_array.size(); ++ix) - (void) get_record_buffer(ix); + m_next_rec_ptr-= (m_record_length - val); } /// Returns total size: pointer array + record buffers. size_t sort_buffer_size() const { - return allocated_size; + return m_size_in_bytes; } - /// Allocates the buffer, but does *not* initialize pointers. - uchar **alloc_sort_buffer(uint num_records, uint record_length); + bool is_allocated() const + { + return m_rawmem; + } + + /** + Allocates the buffer, but does *not* initialize pointers. + Total size = (num_records * record_length) + (num_records * sizeof(pointer)) + space for records space for pointer to records + Caller is responsible for raising an error if allocation fails. + + @param num_records Number of records. + @param record_length (maximum) size of each record. + @returns Pointer to allocated area, or NULL in case of out-of-memory. + */ + uchar *alloc_sort_buffer(uint num_records, uint record_length); /// Frees the buffer. void free_sort_buffer(); - /// Getter, for calling routines which still use the uchar** interface. - uchar **get_sort_keys() { return m_idx_array.array(); } + void reset() + { + m_rawmem= NULL; + } + /** + Used to access the "right-to-left" array of record pointers as an ordinary + "left-to-right" array, so that we can pass it directly on to std::sort(). + */ + uchar **get_sort_keys() + { + if (m_idx == 0) + return NULL; + return &m_record_pointers[1 - m_idx]; + } + + /** + Gets sorted record number ix. @see get_sort_keys() + Only valid after buffer has been sorted! + */ + uchar *get_sorted_record(uint ix) + { + return m_sort_keys[ix]; + } + + /** + @returns The entire buffer, as a character array. + This is for reusing the memory for merge buffers. + */ + Bounds_checked_array<uchar> get_raw_buf() + { + return Bounds_checked_array<uchar>(m_rawmem, m_size_in_bytes); + } /** We need an assignment operator, see filesort(). @@ -117,20 +243,40 @@ class Filesort_buffer */ Filesort_buffer &operator=(const Filesort_buffer &rhs) { - m_idx_array= rhs.m_idx_array; + m_next_rec_ptr= rhs.m_next_rec_ptr; + m_rawmem= rhs.m_rawmem; + m_record_pointers= rhs.m_record_pointers; + m_sort_keys= rhs.m_sort_keys; + m_num_records= rhs.m_num_records; m_record_length= rhs.m_record_length; - m_start_of_data= rhs.m_start_of_data; - allocated_size= rhs.allocated_size; + m_sort_length= rhs.m_sort_length; + m_size_in_bytes= rhs.m_size_in_bytes; + m_idx= rhs.m_idx; return *this; } + uint get_sort_length() const { return m_sort_length; } + void set_sort_length(uint val) { m_sort_length= val; } + private: - typedef Bounds_checked_array<uchar*> Idx_array; + uchar *m_next_rec_ptr; /// The next record will be inserted here. + uchar *m_rawmem; /// The raw memory buffer. + uchar **m_record_pointers; /// The "right-to-left" array of record pointers. + uchar **m_sort_keys; /// Caches the value of get_sort_keys() + uint m_num_records; /// Saved value from alloc_sort_buffer() + uint m_record_length; /// Saved value from alloc_sort_buffer() + uint m_sort_length; /// The length of the sort key. + size_t m_size_in_bytes; /// Size of raw buffer, in bytes. - Idx_array m_idx_array; /* Pointers to key data */ - uint m_record_length; - uchar *m_start_of_data; /* Start of key data */ - size_t allocated_size; + /** + This is the index in the "right-to-left" array of the next record to + be inserted into the buffer. It is signed, because we use it in signed + expressions like: + m_record_pointers[-m_idx]; + It is longlong rather than int, to ensure that it covers UINT_MAX32 + without any casting/warning. + */ + longlong m_idx; }; #endif // FILESORT_UTILS_INCLUDED diff --git a/sql/records.cc b/sql/records.cc index 3d709182a4e..2b146abb005 100644 --- a/sql/records.cc +++ b/sql/records.cc @@ -38,8 +38,8 @@ static int rr_quick(READ_RECORD *info); int rr_sequential(READ_RECORD *info); static int rr_from_tempfile(READ_RECORD *info); -static int rr_unpack_from_tempfile(READ_RECORD *info); -static int rr_unpack_from_buffer(READ_RECORD *info); +template<bool> static int rr_unpack_from_tempfile(READ_RECORD *info); +template<bool> static int rr_unpack_from_buffer(READ_RECORD *info); int rr_from_pointers(READ_RECORD *info); static int rr_from_cache(READ_RECORD *info); static int init_rr_cache(THD *thd, READ_RECORD *info); @@ -187,23 +187,23 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table, bool disable_rr_cache) { IO_CACHE *tempfile; - SORT_ADDON_FIELD *addon_field= filesort ? filesort->addon_field : 0; DBUG_ENTER("init_read_record"); + const bool using_addon_fields= filesort && filesort->using_addon_fields(); + bzero((char*) info,sizeof(*info)); info->thd=thd; info->table=table; - info->addon_field= addon_field; + info->sort_info= filesort; if ((table->s->tmp_table == INTERNAL_TMP_TABLE) && - !addon_field) + !using_addon_fields) (void) table->file->extra(HA_EXTRA_MMAP); - if (addon_field) + if (using_addon_fields) { - info->rec_buf= (uchar*) filesort->addon_buf.str; - info->ref_length= (uint)filesort->addon_buf.length; - info->unpack= filesort->unpack; + info->rec_buf= filesort->addon_fields->get_addon_buf(); + info->ref_length= filesort->addon_fields->get_addon_buf_length(); } else { @@ -223,9 +223,20 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table, if (tempfile && !(select && select->quick)) { - DBUG_PRINT("info",("using rr_from_tempfile")); - info->read_record_func= - addon_field ? rr_unpack_from_tempfile : rr_from_tempfile; + if (using_addon_fields) + { + DBUG_PRINT("info",("using rr_from_tempfile")); + if (filesort->addon_fields->using_packed_addons()) + info->read_record_func= rr_unpack_from_tempfile<true>; + else + info->read_record_func= rr_unpack_from_tempfile<false>; + } + else + { + DBUG_PRINT("info",("using rr_from_tempfile")); + info->read_record_func= rr_from_tempfile; + } + info->io_cache= tempfile; reinit_io_cache(info->io_cache,READ_CACHE,0L,0,0); info->ref_pos=table->file->ref; @@ -239,7 +250,7 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table, and filesort->io_cache is read sequentially */ if (!disable_rr_cache && - !addon_field && + !using_addon_fields && thd->variables.read_rnd_buff_size && !(table->file->ha_table_flags() & HA_FAST_KEY_READ) && (table->db_stat & HA_READ_ONLY || @@ -264,16 +275,29 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table, DBUG_PRINT("info",("using rr_quick")); info->read_record_func= rr_quick; } - else if (filesort && filesort->record_pointers) + else if (filesort && filesort->has_filesort_result_in_memory()) { DBUG_PRINT("info",("using record_pointers")); if (unlikely(table->file->ha_rnd_init_with_error(0))) DBUG_RETURN(1); + info->cache_pos= filesort->record_pointers; - info->cache_end= (info->cache_pos+ - filesort->return_rows * info->ref_length); - info->read_record_func= - addon_field ? rr_unpack_from_buffer : rr_from_pointers; + if (using_addon_fields) + { + DBUG_PRINT("info",("using rr_unpack_from_buffer")); + DBUG_ASSERT(filesort->sorted_result_in_fsbuf); + info->unpack_counter= 0; + if (filesort->using_packed_addons()) + info->read_record_func= rr_unpack_from_buffer<true>; + else + info->read_record_func= rr_unpack_from_buffer<false>; + } + else + { + info->cache_end= (info->cache_pos+ + filesort->return_rows * info->ref_length); + info->read_record_func= rr_from_pointers; + } } else if (table->file->keyread_enabled()) { @@ -510,7 +534,11 @@ static int rr_from_tempfile(READ_RECORD *info) the fields values use in the result set from this buffer into their positions in the regular record buffer. - @param info Reference to the context including record descriptors + @param info Reference to the context including record + descriptors + @param Packed_addon_fields Are the addon fields packed? + This is a compile-time constant, to + avoid if (....) tests during execution. @retval 0 Record successfully read. @@ -518,12 +546,38 @@ static int rr_from_tempfile(READ_RECORD *info) -1 There is no record to be read anymore. */ +template<bool Packed_addon_fields> static int rr_unpack_from_tempfile(READ_RECORD *info) { - if (my_b_read(info->io_cache, info->rec_buf, info->ref_length)) - return -1; - (*info->unpack)(info->addon_field, info->rec_buf, - info->rec_buf + info->ref_length); + uchar *destination= info->rec_buf; +#ifndef DBUG_OFF + my_off_t where= my_b_tell(info->io_cache); +#endif + if (Packed_addon_fields) + { + const uint len_sz= Addon_fields::size_of_length_field; + + // First read length of the record. + if (my_b_read(info->io_cache, destination, len_sz)) + return -1; + uint res_length= Addon_fields::read_addon_length(destination); + DBUG_PRINT("info", ("rr_unpack from %llu to %p sz %u", + static_cast<ulonglong>(where), + destination, res_length)); + DBUG_ASSERT(res_length > len_sz); + DBUG_ASSERT(info->sort_info->using_addon_fields()); + + // Then read the rest of the record. + if (my_b_read(info->io_cache, destination + len_sz, res_length - len_sz)) + return -1; /* purecov: inspected */ + } + else + { + if (my_b_read(info->io_cache, destination, info->ref_length)) + return -1; + } + + info->sort_info->unpack_addon_fields<Packed_addon_fields>(destination); return 0; } @@ -560,7 +614,11 @@ int rr_from_pointers(READ_RECORD *info) the fields values use in the result set from this buffer into their positions in the regular record buffer. - @param info Reference to the context including record descriptors + @param info Reference to the context including record + descriptors + @param Packed_addon_fields Are the addon fields packed? + This is a compile-time constant, to + avoid if (....) tests during execution. @retval 0 Record successfully read. @@ -568,13 +626,17 @@ int rr_from_pointers(READ_RECORD *info) -1 There is no record to be read anymore. */ +template<bool Packed_addon_fields> static int rr_unpack_from_buffer(READ_RECORD *info) { - if (info->cache_pos == info->cache_end) + if (info->unpack_counter == info->sort_info->return_rows) return -1; /* End of buffer */ - (*info->unpack)(info->addon_field, info->cache_pos, - info->cache_end); - info->cache_pos+= info->ref_length; + + uchar *record= info->sort_info->get_sorted_record( + static_cast<uint>(info->unpack_counter)); + uchar *plen= record + info->sort_info->get_sort_length(); + info->sort_info->unpack_addon_fields<Packed_addon_fields>(plen); + info->unpack_counter++; return 0; } /* cacheing of records from a database */ @@ -709,3 +771,26 @@ static int rr_cmp(uchar *a,uchar *b) return (int) a[7] - (int) b[7]; #endif } + +template<bool Packed_addon_fields> +inline void SORT_INFO::unpack_addon_fields(uchar *buff) +{ + SORT_ADDON_FIELD *addonf= addon_fields->begin(); + uchar *buff_end= buff + sort_buffer_size(); + const uchar *start_of_record= buff + addonf->offset; + + for ( ; addonf != addon_fields->end() ; addonf++) + { + Field *field= addonf->field; + if (addonf->null_bit && (addonf->null_bit & buff[addonf->null_offset])) + { + field->set_null(); + continue; + } + field->set_notnull(); + if (Packed_addon_fields) + start_of_record= field->unpack(field->ptr, start_of_record, buff_end, 0); + else + field->unpack(field->ptr, buff + addonf->offset, buff_end, 0); + } +} diff --git a/sql/records.h b/sql/records.h index faf0d13c9a9..04dc06b3c74 100644 --- a/sql/records.h +++ b/sql/records.h @@ -58,13 +58,23 @@ struct READ_RECORD THD *thd; SQL_SELECT *select; uint ref_length, reclength, rec_cache_size, error_offset; + + /** + Counting records when reading result from filesort(). + Used when filesort leaves the result in the filesort buffer. + */ + ha_rows unpack_counter; + uchar *ref_pos; /* pointer to form->refpos */ uchar *rec_buf; /* to read field values after filesort */ uchar *cache,*cache_pos,*cache_end,*read_positions; - struct st_sort_addon_field *addon_field; /* Pointer to the fields info */ + + /* + Structure storing information about sorting + */ + SORT_INFO *sort_info; struct st_io_cache *io_cache; bool print_error; - void (*unpack)(struct st_sort_addon_field *, uchar *, uchar *); int read_record() { return read_record_func(this); } uchar *record() const { return table->record[0]; } diff --git a/sql/sql_array.h b/sql/sql_array.h index bcfbb98ef19..b05e8f779bd 100644 --- a/sql/sql_array.h +++ b/sql/sql_array.h @@ -85,6 +85,10 @@ template <typename Element_type> class Bounds_checked_array Element_type *array() const { return m_array; } + Element_type *begin() const { return array(); } + Element_type *end() const { return array() + m_size; } + + bool operator==(const Bounds_checked_array<Element_type>&rhs) const { return m_array == rhs.m_array && m_size == rhs.m_size; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 3619c603697..4da2eb3a4d4 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -13981,7 +13981,7 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond, *simple_order= head->on_expr_ref[0] == NULL; if (*simple_order && head->table->file->ha_table_flags() & HA_SLOW_RND_POS) { - uint u1, u2, u3; + uint u1, u2, u3, u4; /* normally the condition is (see filesort_use_addons()) @@ -13992,7 +13992,7 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond, TODO proper cost estimations */ - *simple_order= filesort_use_addons(head->table, 0, &u1, &u2, &u3); + *simple_order= filesort_use_addons(head->table, 0, &u1, &u2, &u3, &u4); } } else diff --git a/sql/sql_sort.h b/sql/sql_sort.h index 7abbc808632..5aa8f4545a4 100644 --- a/sql/sql_sort.h +++ b/sql/sql_sort.h @@ -20,8 +20,6 @@ #include <my_sys.h> /* qsort2_cmp */ #include "queues.h" -typedef struct st_buffpek BUFFPEK; - struct SORT_FIELD; class Field; struct TABLE; @@ -64,21 +62,236 @@ struct BUFFPEK_COMPARE_CONTEXT }; +/** + Descriptor for a merge chunk to be sort-merged. + A merge chunk is a sequence of pre-sorted records, written to a + temporary file. A Merge_chunk instance describes where this chunk is stored + in the file, and where it is located when it is in memory. + + It is a POD because + - we read/write them from/to files. + + We have accessors (getters/setters) for all struct members. + */ + +struct Merge_chunk { +public: + Merge_chunk(): m_current_key(NULL), + m_file_position(0), + m_buffer_start(NULL), + m_buffer_end(NULL), + m_rowcount(0), + m_mem_count(0), + m_max_keys(0) + {} + + my_off_t file_position() const { return m_file_position; } + void set_file_position(my_off_t val) { m_file_position= val; } + void advance_file_position(my_off_t val) { m_file_position+= val; } + + uchar *buffer_start() { return m_buffer_start; } + const uchar *buffer_end() const { return m_buffer_end; } + + void set_buffer(uchar *start, uchar *end) + { + m_buffer_start= start; + m_buffer_end= end; + } + void set_buffer_start(uchar *start) + { + m_buffer_start= start; + } + void set_buffer_end(uchar *end) + { + DBUG_ASSERT(m_buffer_end == NULL || end <= m_buffer_end); + m_buffer_end= end; + } + + void init_current_key() { m_current_key= m_buffer_start; } + uchar *current_key() { return m_current_key; } + void advance_current_key(uint val) { m_current_key+= val; } + + void decrement_rowcount(ha_rows val) { m_rowcount-= val; } + void set_rowcount(ha_rows val) { m_rowcount= val; } + ha_rows rowcount() const { return m_rowcount; } + + ha_rows mem_count() const { return m_mem_count; } + void set_mem_count(ha_rows val) { m_mem_count= val; } + ha_rows decrement_mem_count() { return --m_mem_count; } + + ha_rows max_keys() const { return m_max_keys; } + void set_max_keys(ha_rows val) { m_max_keys= val; } + + size_t buffer_size() const { return m_buffer_end - m_buffer_start; } + + /** + Tries to merge *this with *mc, returns true if successful. + The assumption is that *this is no longer in use, + and the space it has been allocated can be handed over to a + buffer which is adjacent to it. + */ + bool merge_freed_buff(Merge_chunk *mc) const + { + if (mc->m_buffer_end == m_buffer_start) + { + mc->m_buffer_end= m_buffer_end; + mc->m_max_keys+= m_max_keys; + return true; + } + else if (mc->m_buffer_start == m_buffer_end) + { + mc->m_buffer_start= m_buffer_start; + mc->m_max_keys+= m_max_keys; + return true; + } + return false; + } + + uchar *m_current_key; /// The current key for this chunk. + my_off_t m_file_position;/// Current position in the file to be sorted. + uchar *m_buffer_start; /// Start of main-memory buffer for this chunk. + uchar *m_buffer_end; /// End of main-memory buffer for this chunk. + ha_rows m_rowcount; /// Number of unread rows in this chunk. + ha_rows m_mem_count; /// Number of rows in the main-memory buffer. + ha_rows m_max_keys; /// If we have fixed-size rows: + /// max number of rows in buffer. +}; + +typedef Bounds_checked_array<SORT_ADDON_FIELD> Addon_fields_array; + +/** + This class wraps information about usage of addon fields. + An Addon_fields object is used both during packing of data in the filesort + buffer, and later during unpacking in 'Filesort_info::unpack_addon_fields'. + + @see documentation for the Sort_addon_field struct. + @see documentation for get_addon_fields() + */ +class Addon_fields { +public: + Addon_fields(Addon_fields_array arr) + : m_field_descriptors(arr), + m_addon_buf(), + m_addon_buf_length(), + m_using_packed_addons(false) + { + DBUG_ASSERT(!arr.is_null()); + } + + SORT_ADDON_FIELD *begin() { return m_field_descriptors.begin(); } + SORT_ADDON_FIELD *end() { return m_field_descriptors.end(); } + + /// rr_unpack_from_tempfile needs an extra buffer when unpacking. + uchar *allocate_addon_buf(uint sz) + { + m_addon_buf= (uchar *)my_malloc(sz, MYF(MY_WME | MY_THREAD_SPECIFIC)); + if (m_addon_buf) + m_addon_buf_length= sz; + return m_addon_buf; + } + + void free_addon_buff() + { + my_free(m_addon_buf); + m_addon_buf= NULL; + m_addon_buf_length= 0; + } + + uchar *get_addon_buf() { return m_addon_buf; } + uint get_addon_buf_length() const { return m_addon_buf_length; } + + void set_using_packed_addons(bool val) + { + m_using_packed_addons= val; + } + + bool using_packed_addons() const + { + return m_using_packed_addons; + } + + static bool can_pack_addon_fields(uint record_length) + { + return (record_length <= (0xFFFF)); + } + + /** + @returns Total number of bytes used for packed addon fields. + the size of the length field + size of null bits + sum of field sizes. + */ + static uint read_addon_length(uchar *p) + { + return size_of_length_field + uint2korr(p); + } + + /** + Stores the number of bytes used for packed addon fields. + */ + static void store_addon_length(uchar *p, uint sz) + { + // We actually store the length of everything *after* the length field. + int2store(p, sz - size_of_length_field); + } + + static const uint size_of_length_field= 2; + +private: + Addon_fields_array m_field_descriptors; + + uchar *m_addon_buf; ///< Buffer for unpacking addon fields. + uint m_addon_buf_length; ///< Length of the buffer. + bool m_using_packed_addons; ///< Are we packing the addon fields? +}; + + +/** + There are two record formats for sorting: + |<key a><key b>...|<rowid>| + / sort_length / ref_l / + + or with "addon fields" + |<key a><key b>...|<null bits>|<field a><field b>...| + / sort_length / addon_length / + + The packed format for "addon fields" + |<key a><key b>...|<length>|<null bits>|<field a><field b>...| + / sort_length / addon_length / + + <key> Fields are fixed-size, specially encoded with + Field::make_sort_key() so we can do byte-by-byte compare. + <length> Contains the *actual* packed length (after packing) of + everything after the sort keys. + The size of the length field is 2 bytes, + which should cover most use cases: addon data <= 65535 bytes. + This is the same as max record size in MySQL. + <null bits> One bit for each nullable field, indicating whether the field + is null or not. May have size zero if no fields are nullable. + <field xx> Are stored with field->pack(), and retrieved with + field->unpack(). Addon fields within a record are stored + consecutively, with no "holes" or padding. They will have zero + size for NULL values. + +*/ + class Sort_param { public: uint rec_length; // Length of sorted records. uint sort_length; // Length of sorted columns. uint ref_length; // Length of record ref. + uint addon_length; // Length of addon_fields uint res_length; // Length of records in final sorted file/buffer. uint max_keys_per_buffer; // Max keys / buffer. uint min_dupl_count; ha_rows max_rows; // Select limit, or HA_POS_ERROR if unlimited. ha_rows examined_rows; // Number of examined rows. TABLE *sort_form; // For quicker make_sortkey. - SORT_FIELD *local_sortorder; - SORT_FIELD *end; - SORT_ADDON_FIELD *addon_field; // Descriptors for companion fields. - LEX_STRING addon_buf; // Buffer & length of added packed fields. + /** + ORDER BY list with some precalculated info for filesort. + Array is created and owned by a Filesort instance. + */ + Bounds_checked_array<SORT_FIELD> local_sortorder; + Addon_fields *addon_fields; // Descriptors for companion fields. + bool using_pq; uchar *unique_buff; bool not_killable; @@ -93,21 +306,63 @@ class Sort_param { } void init_for_filesort(uint sortlen, TABLE *table, ha_rows maxrows, bool sort_positions); + /// Enables the packing of addons if possible. + void try_to_pack_addons(ulong max_length_for_sort_data); + + /// Are we packing the "addon fields"? + bool using_packed_addons() const + { + DBUG_ASSERT(m_using_packed_addons == + (addon_fields != NULL && + addon_fields->using_packed_addons())); + return m_using_packed_addons; + } + + /// Are we using "addon fields"? + bool using_addon_fields() const + { + return addon_fields != NULL; + } + + /** + Getter for record length and result length. + @param record_start Pointer to record. + @param [out] recl Store record length here. + @param [out] resl Store result length here. + */ + void get_rec_and_res_len(uchar *record_start, uint *recl, uint *resl) + { + if (!using_packed_addons()) + { + *recl= rec_length; + *resl= res_length; + return; + } + uchar *plen= record_start + sort_length; + *resl= Addon_fields::read_addon_length(plen); + DBUG_ASSERT(*resl <= res_length); + const uchar *record_end= plen + *resl; + *recl= static_cast<uint>(record_end - record_start); + } + +private: + uint m_packable_length; + bool m_using_packed_addons; ///< caches the value of using_packed_addons() }; +typedef Bounds_checked_array<uchar> Sort_buffer; -int merge_many_buff(Sort_param *param, uchar *sort_buffer, - BUFFPEK *buffpek, - uint *maxbuffer, IO_CACHE *t_file); -ulong read_to_buffer(IO_CACHE *fromfile,BUFFPEK *buffpek, - uint sort_length); +int merge_many_buff(Sort_param *param, Sort_buffer sort_buffer, + Merge_chunk *buffpek, uint *maxbuffer, IO_CACHE *t_file); +ulong read_to_buffer(IO_CACHE *fromfile, Merge_chunk *buffpek, + Sort_param *param); bool merge_buffers(Sort_param *param,IO_CACHE *from_file, - IO_CACHE *to_file, uchar *sort_buffer, - BUFFPEK *lastbuff,BUFFPEK *Fb, - BUFFPEK *Tb,int flag); -int merge_index(Sort_param *param, uchar *sort_buffer, - BUFFPEK *buffpek, uint maxbuffer, - IO_CACHE *tempfile, IO_CACHE *outfile); -void reuse_freed_buff(QUEUE *queue, BUFFPEK *reuse, uint key_length); + IO_CACHE *to_file, Sort_buffer sort_buffer, + Merge_chunk *lastbuff, Merge_chunk *Fb, + Merge_chunk *Tb, int flag); +int merge_index(Sort_param *param, Sort_buffer sort_buffer, + Merge_chunk *buffpek, uint maxbuffer, + IO_CACHE *tempfile, IO_CACHE *outfile); +void reuse_freed_buff(QUEUE *queue, Merge_chunk *reuse, uint key_length); #endif /* SQL_SORT_INCLUDED */ diff --git a/sql/uniques.cc b/sql/uniques.cc index fafb44b56a0..a8170951e88 100644 --- a/sql/uniques.cc +++ b/sql/uniques.cc @@ -39,7 +39,6 @@ #include "my_tree.h" // element_count #include "uniques.h" // Unique #include "sql_sort.h" -#include "myisamchk.h" // BUFFPEK int unique_write_to_file(uchar* key, element_count count, Unique *unique) { @@ -94,7 +93,7 @@ Unique::Unique(qsort_cmp2 comp_func, void * comp_func_fixed_arg, init_tree(&tree, (max_in_memory_size / 16), 0, size, comp_func, NULL, comp_func_fixed_arg, MYF(MY_THREAD_SPECIFIC)); /* If the following fail's the next add will also fail */ - my_init_dynamic_array(&file_ptrs, sizeof(BUFFPEK), 16, 16, + my_init_dynamic_array(&file_ptrs, sizeof(Merge_chunk), 16, 16, MYF(MY_THREAD_SPECIFIC)); /* If you change the following, change it in get_max_elements function, too. @@ -375,10 +374,10 @@ Unique::~Unique() /* Write tree to disk; clear tree */ bool Unique::flush() { - BUFFPEK file_ptr; + Merge_chunk file_ptr; elements+= tree.elements_in_tree; - file_ptr.count=tree.elements_in_tree; - file_ptr.file_pos=my_b_tell(&file); + file_ptr.set_rowcount(tree.elements_in_tree); + file_ptr.set_file_position(my_b_tell(&file)); tree_walk_action action= min_dupl_count ? (tree_walk_action) unique_write_to_file_with_count : @@ -490,7 +489,7 @@ void put_counter_into_merged_element(void *ptr, uint ofs, element_count cnt) */ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, - uint key_length, BUFFPEK *begin, BUFFPEK *end, + uint key_length, Merge_chunk *begin, Merge_chunk *end, tree_walk_action walk_action, void *walk_action_arg, qsort_cmp2 compare, void *compare_arg, IO_CACHE *file, bool with_counters) @@ -499,7 +498,8 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, QUEUE queue; if (end <= begin || merge_buffer_size < (size_t) (key_length * (end - begin + 1)) || - init_queue(&queue, (uint) (end - begin), offsetof(BUFFPEK, key), 0, + init_queue(&queue, (uint) (end - begin), + offsetof(Merge_chunk, m_current_key), 0, buffpek_compare, &compare_context, 0, 0)) return 1; /* we need space for one key when a piece of merge buffer is re-read */ @@ -510,10 +510,16 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, /* if piece_size is aligned reuse_freed_buffer will always hit */ uint piece_size= max_key_count_per_piece * key_length; ulong bytes_read; /* to hold return value of read_to_buffer */ - BUFFPEK *top; + Merge_chunk *top; int res= 1; uint cnt_ofs= key_length - (with_counters ? sizeof(element_count) : 0); element_count cnt; + + // read_to_buffer() needs only rec_length. + Sort_param sort_param; + sort_param.rec_length= key_length; + DBUG_ASSERT(!sort_param.using_addon_fields()); + /* Invariant: queue must contain top element from each tree, until a tree is not completely walked through. @@ -522,15 +528,16 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, */ for (top= begin; top != end; ++top) { - top->base= merge_buffer + (top - begin) * piece_size; - top->max_keys= max_key_count_per_piece; - bytes_read= read_to_buffer(file, top, key_length); + top->set_buffer_start(merge_buffer + (top - begin) * piece_size); + top->set_buffer_end(top->buffer_start() + piece_size); + top->set_max_keys(max_key_count_per_piece); + bytes_read= read_to_buffer(file, top, &sort_param); if (unlikely(bytes_read == (ulong) -1)) goto end; DBUG_ASSERT(bytes_read); queue_insert(&queue, (uchar *) top); } - top= (BUFFPEK *) queue_top(&queue); + top= (Merge_chunk *) queue_top(&queue); while (queue.elements > 1) { /* @@ -540,20 +547,21 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, elements in each tree are unique. Action is applied only to unique elements. */ - void *old_key= top->key; + void *old_key= top->current_key(); /* read next key from the cache or from the file and push it to the queue; this gives new top. */ - top->key+= key_length; - if (--top->mem_count) + top->advance_current_key(key_length); + top->decrement_mem_count(); + if (top->mem_count()) queue_replace_top(&queue); else /* next piece should be read */ { /* save old_key not to overwrite it in read_to_buffer */ memcpy(save_key_buff, old_key, key_length); old_key= save_key_buff; - bytes_read= read_to_buffer(file, top, key_length); + bytes_read= read_to_buffer(file, top, &sort_param); if (unlikely(bytes_read == (ulong) -1)) goto end; else if (bytes_read) /* top->key, top->mem_count are reset */ @@ -568,9 +576,9 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, reuse_freed_buff(&queue, top, key_length); } } - top= (BUFFPEK *) queue_top(&queue); + top= (Merge_chunk *) queue_top(&queue); /* new top has been obtained; if old top is unique, apply the action */ - if (compare(compare_arg, old_key, top->key)) + if (compare(compare_arg, old_key, top->current_key())) { cnt= with_counters ? get_counter_from_merged_element(old_key, cnt_ofs) : 1; @@ -579,9 +587,9 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, } else if (with_counters) { - cnt= get_counter_from_merged_element(top->key, cnt_ofs); + cnt= get_counter_from_merged_element(top->current_key(), cnt_ofs); cnt+= get_counter_from_merged_element(old_key, cnt_ofs); - put_counter_into_merged_element(top->key, cnt_ofs, cnt); + put_counter_into_merged_element(top->current_key(), cnt_ofs, cnt); } } /* @@ -595,13 +603,13 @@ static bool merge_walk(uchar *merge_buffer, size_t merge_buffer_size, { cnt= with_counters ? - get_counter_from_merged_element(top->key, cnt_ofs) : 1; - if (walk_action(top->key, cnt, walk_action_arg)) + get_counter_from_merged_element(top->current_key(), cnt_ofs) : 1; + if (walk_action(top->current_key(), cnt, walk_action_arg)) goto end; - top->key+= key_length; + top->advance_current_key(key_length); } - while (--top->mem_count); - bytes_read= read_to_buffer(file, top, key_length); + while (top->decrement_mem_count()); + bytes_read= read_to_buffer(file, top, &sort_param); if (unlikely(bytes_read == (ulong) -1)) goto end; } @@ -657,13 +665,14 @@ bool Unique::walk(TABLE *table, tree_walk_action action, void *walk_action_arg) if (!(merge_buffer = (uchar *)my_malloc(buff_sz, MYF(MY_WME)))) return 1; if (buff_sz < full_size * (file_ptrs.elements + 1UL)) - res= merge(table, merge_buffer, buff_sz >= full_size * MERGEBUFF2) ; + res= merge(table, merge_buffer, buff_sz, + buff_sz >= full_size * MERGEBUFF2) ; if (!res) { res= merge_walk(merge_buffer, buff_sz, full_size, - (BUFFPEK *) file_ptrs.buffer, - (BUFFPEK *) file_ptrs.buffer + file_ptrs.elements, + (Merge_chunk *) file_ptrs.buffer, + (Merge_chunk *) file_ptrs.buffer + file_ptrs.elements, action, walk_action_arg, tree.compare, tree.custom_arg, &file, with_counters); } @@ -684,16 +693,18 @@ bool Unique::walk(TABLE *table, tree_walk_action action, void *walk_action_arg) All params are 'IN': table the parameter to access sort context buff merge buffer + buff_size size of merge buffer without_last_merge TRUE <=> do not perform the last merge RETURN VALUE 0 OK <> 0 error */ -bool Unique::merge(TABLE *table, uchar *buff, bool without_last_merge) +bool Unique::merge(TABLE *table, uchar *buff, size_t buff_size, + bool without_last_merge) { IO_CACHE *outfile= &sort.io_cache; - BUFFPEK *file_ptr= (BUFFPEK*) file_ptrs.buffer; + Merge_chunk *file_ptr= (Merge_chunk*) file_ptrs.buffer; uint maxbuffer= file_ptrs.elements - 1; my_off_t save_pos; bool error= 1; @@ -724,7 +735,9 @@ bool Unique::merge(TABLE *table, uchar *buff, bool without_last_merge) sort_param.cmp_context.key_compare_arg= tree.custom_arg; /* Merge the buffers to one file, removing duplicates */ - if (merge_many_buff(&sort_param,buff,file_ptr,&maxbuffer,&file)) + if (merge_many_buff(&sort_param, + Bounds_checked_array<uchar>(buff, buff_size), + file_ptr,&maxbuffer,&file)) goto err; if (flush_io_cache(&file) || reinit_io_cache(&file,READ_CACHE,0L,0,0)) @@ -736,7 +749,8 @@ bool Unique::merge(TABLE *table, uchar *buff, bool without_last_merge) file_ptrs.elements= maxbuffer+1; return 0; } - if (merge_index(&sort_param, buff, file_ptr, maxbuffer, &file, outfile)) + if (merge_index(&sort_param, Bounds_checked_array<uchar>(buff, buff_size), + file_ptr, maxbuffer, &file, outfile)) goto err; error= 0; err: @@ -791,7 +805,7 @@ bool Unique::get(TABLE *table) MYF(MY_THREAD_SPECIFIC|MY_WME)))) DBUG_RETURN(1); - if (merge(table, sort_buffer, FALSE)) + if (merge(table, sort_buffer, buff_sz, FALSE)) goto err; rc= 0; diff --git a/sql/uniques.h b/sql/uniques.h index 654b3692aaa..f83eac36855 100644 --- a/sql/uniques.h +++ b/sql/uniques.h @@ -39,7 +39,7 @@ class Unique :public Sql_alloc uint min_dupl_count; /* always 0 for unions, > 0 for intersections */ bool with_counters; - bool merge(TABLE *table, uchar *buff, bool without_last_merge); + bool merge(TABLE *table, uchar *buff, size_t size, bool without_last_merge); bool flush(); public: diff --git a/storage/connect/mysql-test/connect/r/mysql_index.result b/storage/connect/mysql-test/connect/r/mysql_index.result index b0c88b16fef..5f8f41f6218 100644 --- a/storage/connect/mysql-test/connect/r/mysql_index.result +++ b/storage/connect/mysql-test/connect/r/mysql_index.result @@ -299,11 +299,11 @@ matricule nom prenom 7626 HENIN PHILIPPE 403 HERMITTE PHILIPPE 9096 HELENA PHILIPPE -SELECT matricule, nom, prenom FROM t2 ORDER BY nom LIMIT 10; +SELECT matricule, nom, prenom FROM t2 ORDER BY nom,prenom LIMIT 10; matricule nom prenom 4552 ABBADIE MONIQUE -6627 ABBAYE GERALD 307 ABBAYE ANNICK +6627 ABBAYE GERALD 7961 ABBE KATIA 1340 ABBE MICHELE 9270 ABBE SOPHIE diff --git a/storage/connect/mysql-test/connect/t/mysql_index.test b/storage/connect/mysql-test/connect/t/mysql_index.test index 74dc48f42c8..e36a827ac3c 100644 --- a/storage/connect/mysql-test/connect/t/mysql_index.test +++ b/storage/connect/mysql-test/connect/t/mysql_index.test @@ -120,7 +120,7 @@ SELECT matricule, nom, prenom FROM t2 WHERE nom <= 'ABEL' OR nom > 'YVON'; SELECT matricule, nom, prenom FROM t2 WHERE nom > 'HELEN' AND nom < 'HEROS'; SELECT matricule, nom, prenom FROM t2 WHERE nom BETWEEN 'HELEN' AND 'HEROS'; SELECT matricule, nom, prenom FROM t2 WHERE nom BETWEEN 'HELEN' AND 'HEROS' AND prenom = 'PHILIPPE'; -SELECT matricule, nom, prenom FROM t2 ORDER BY nom LIMIT 10; +SELECT matricule, nom, prenom FROM t2 ORDER BY nom,prenom LIMIT 10; SELECT a.nom, a.prenom, b.nom FROM t1 a STRAIGHT_JOIN t2 b ON a.prenom = b.prenom WHERE a.nom = 'FOCH' AND a.nom != b.nom; DROP TABLE t2;

1 0

[Commits] 359d91aaeec: MDEV-19680:: Assertion `!table || (!table->read_set || bitmap_is_set(table->read_set, field_index) || (!(ptr >= table->record[0] && ptr < table->record[0] + table->s->reclength)))' or alike failed upon SELECT with mix of functions from simple view
by Varun 26 Dec '19

26 Dec '19

revision-id: 359d91aaeec25825b51b0a00f52f272edad7d6cc (mariadb-10.1.39-254-g359d91aaeec) parent(s): 9f7fcb9f25238945e4fb8cc1a1f98e56457b714f author: Varun Gupta committer: Varun Gupta timestamp: 2019-12-26 17:36:32 +0530 message: MDEV-19680:: Assertion `!table || (!table->read_set || bitmap_is_set(table->read_set, field_index) || (!(ptr >= table->record[0] && ptr < table->record[0] + table->s->reclength)))' or alike failed upon SELECT with mix of functions from simple view Set read_set bitmap for view from the JOIN::all_fields list instead of JOIN::fields_list as split_sum_func would have added items to the all_fields list. --- mysql-test/r/func_misc.result | 13 +++++++++++++ mysql-test/t/func_misc.test | 15 +++++++++++++++ sql/sql_lex.cc | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/func_misc.result b/mysql-test/r/func_misc.result index 287a70f1f73..89f6102ef83 100644 --- a/mysql-test/r/func_misc.result +++ b/mysql-test/r/func_misc.result @@ -1479,3 +1479,16 @@ EXECUTE stmt; x x DEALLOCATE PREPARE stmt; +# +# MDEV-19680: Assertion `!table || (!table->read_set || bitmap_is_set(table->read_set, field_index) || +# (!(ptr >= table->record[0] && ptr < table->record[0] + table->s->reclength)))' +# or alike failed upon SELECT with mix of functions from simple view +# +CREATE TABLE t1 (a INT) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1),(2); +CREATE VIEW v1 AS SELECT * FROM t1; +SELECT ISNULL( BENCHMARK(1, MIN(a))) FROM v1; +ISNULL( BENCHMARK(1, MIN(a))) +0 +DROP VIEW v1; +DROP TABLE t1; diff --git a/mysql-test/t/func_misc.test b/mysql-test/t/func_misc.test index a8da9068ab8..6412980a5fa 100644 --- a/mysql-test/t/func_misc.test +++ b/mysql-test/t/func_misc.test @@ -1154,3 +1154,18 @@ DROP PROCEDURE p1; PREPARE stmt FROM "SELECT 'x' ORDER BY NAME_CONST( 'f', 'foo' )"; EXECUTE stmt; DEALLOCATE PREPARE stmt; + +--echo # +--echo # MDEV-19680: Assertion `!table || (!table->read_set || bitmap_is_set(table->read_set, field_index) || +--echo # (!(ptr >= table->record[0] && ptr < table->record[0] + table->s->reclength)))' +--echo # or alike failed upon SELECT with mix of functions from simple view +--echo # + +CREATE TABLE t1 (a INT) ENGINE=MyISAM; +INSERT INTO t1 VALUES (1),(2); +CREATE VIEW v1 AS SELECT * FROM t1; + +SELECT ISNULL( BENCHMARK(1, MIN(a))) FROM v1; + +DROP VIEW v1; +DROP TABLE t1; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index a36a19357eb..1cd2a369d7a 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4177,7 +4177,7 @@ void SELECT_LEX::update_used_tables() } Item *item; - List_iterator_fast<Item> it(join->fields_list); + List_iterator_fast<Item> it(join->all_fields); while ((item= it++)) { item->update_used_tables();

1 0