Skip to content

Commit afd7820

Browse files
author
Steinar H. Gunderson
committed
WL #13000: Iterator UNION [patch 16/16; rework PFS batch mode]
Rework the way we turn on and off performance schema batch mode in iterators. The signal to turn on and off is now exclusively sent through iterators, instead of looking into QEP_TABs and cleaning up directly on handlers. In particular, this means that we send the “end PFS batch mode” signal to every single iterator in the tree at the end, guaranteeing that no involved table is left behind. This is a bit more brute-forcish than trying to turn it off on only the ones where we've turned it on, but much, much simpler to get right. The method for starting batch mode is also changed; instead of trying to figure out from the outside whether NestedLoopIterator is involved or not, we call StartPSIBatchMode() on the root iterator, which will send the signal down to the table iterator if we have a single table; if not, it will be stopped and ignored at the first NestedLoopIterator. Again, this vastly simplifies the logic. Change-Id: I8ad0e11e0fc0db6a54174c1b841c103f57fab32d
1 parent a4dffb2 commit afd7820

10 files changed

+233
-159
lines changed

Diff for: ‎mysql-test/r/insert.result

+17
Original file line numberDiff line numberDiff line change
@@ -1001,3 +1001,20 @@ ERROR 42000: Column 'a' specified twice
10011001
INSERT INTO t1(b, b) VALUES (DEFAULT, DEFAULT);
10021002
ERROR 42000: Column 'b' specified twice
10031003
DROP TABLE t1;
1004+
#
1005+
# Bug #29899614: SIG 6 -`M_PSI_BATCH_MODE == PSI_BATCH_MODE_NONE' | SQL/HANDLER.CC
1006+
#
1007+
CREATE TABLE t1 ( f1 INTEGER, INDEX ( f1 ) );
1008+
CREATE TABLE t2 ( f1 INTEGER );
1009+
INSERT INTO t1 VALUES (10);
1010+
INSERT INTO t2
1011+
SELECT STRAIGHT_JOIN *
1012+
FROM t1 AS alias1
1013+
WHERE EXISTS (
1014+
SELECT * FROM (
1015+
SELECT * FROM t1 JOIN t1 AS alias2 USING ( f1 )
1016+
) AS alias3
1017+
WHERE alias1.f1 < 20
1018+
);
1019+
FLUSH TABLES;
1020+
DROP TABLE t1, t2;

Diff for: ‎mysql-test/t/insert.test

+25
Original file line numberDiff line numberDiff line change
@@ -907,3 +907,28 @@ INSERT INTO t1(a, a, b, c) VALUES (1, 1, DEFAULT, DEFAULT);
907907
INSERT INTO t1(b, b) VALUES (DEFAULT, DEFAULT);
908908

909909
DROP TABLE t1;
910+
911+
--echo #
912+
--echo # Bug #29899614: SIG 6 -`M_PSI_BATCH_MODE == PSI_BATCH_MODE_NONE' | SQL/HANDLER.CC
913+
--echo #
914+
915+
CREATE TABLE t1 ( f1 INTEGER, INDEX ( f1 ) );
916+
CREATE TABLE t2 ( f1 INTEGER );
917+
918+
INSERT INTO t1 VALUES (10);
919+
920+
INSERT INTO t2
921+
SELECT STRAIGHT_JOIN *
922+
FROM t1 AS alias1
923+
WHERE EXISTS (
924+
SELECT * FROM (
925+
SELECT * FROM t1 JOIN t1 AS alias2 USING ( f1 )
926+
) AS alias3
927+
WHERE alias1.f1 < 20
928+
);
929+
930+
# We don't care about the result, but close the tables to make sure that
931+
# we haven't inadvertedly left any of them in performance schema batch mode.
932+
FLUSH TABLES;
933+
934+
DROP TABLE t1, t2;

Diff for: ‎sql/composite_iterators.cc

+31-15
Original file line numberDiff line numberDiff line change
@@ -503,13 +503,6 @@ int AggregateIterator::Read() {
503503
return 1;
504504
}
505505

506-
void AggregateIterator::UnlockRow() {
507-
// Most likely, HAVING failed. Ideally, we'd like to backtrack and
508-
// unlock all rows that went into this aggregate, but we can't do that,
509-
// and we also can't unlock the _current_ row, since that belongs to a
510-
// different group. Thus, do nothing.
511-
}
512-
513506
vector<string> AggregateIterator::DebugString() const {
514507
Item_sum **sum_funcs_end =
515508
m_rollup ? m_join->sum_funcs_end[m_join->send_group_parts]
@@ -562,10 +555,6 @@ int PrecomputedAggregateIterator::Read() {
562555
return 0;
563556
}
564557

565-
void PrecomputedAggregateIterator::UnlockRow() {
566-
// See AggregateIterator::UnlockRow().
567-
}
568-
569558
vector<string> PrecomputedAggregateIterator::DebugString() const {
570559
string ret;
571560

@@ -598,7 +587,9 @@ bool NestedLoopIterator::Init() {
598587
return true;
599588
}
600589
m_state = NEEDS_OUTER_ROW;
601-
m_source_inner->EndPSIBatchModeIfStarted();
590+
if (m_pfs_batch_mode) {
591+
m_source_inner->EndPSIBatchModeIfStarted();
592+
}
602593
return false;
603594
}
604595

@@ -630,7 +621,7 @@ int NestedLoopIterator::Read() {
630621
m_state == READING_FIRST_INNER_ROW);
631622

632623
int err = m_source_inner->Read();
633-
if (err != 0) {
624+
if (err != 0 && m_pfs_batch_mode) {
634625
m_source_inner->EndPSIBatchModeIfStarted();
635626
}
636627
if (err == 1) {
@@ -1022,7 +1013,7 @@ bool MaterializeIterator::MaterializeQueryBlock(const QueryBlock &query_block,
10221013
return true;
10231014
}
10241015

1025-
PFSBatchMode pfs_batch_mode(nullptr, join);
1016+
PFSBatchMode pfs_batch_mode(query_block.subquery_iterator.get());
10261017
while (*stored_rows < m_limit_rows) {
10271018
int error = query_block.subquery_iterator->Read();
10281019
if (error > 0 || thd()->is_error())
@@ -1214,6 +1205,13 @@ vector<RowIterator::Child> MaterializeIterator::children() const {
12141205
return ret;
12151206
}
12161207

1208+
void MaterializeIterator::EndPSIBatchModeIfStarted() {
1209+
for (const QueryBlock &query_block : m_query_blocks_to_materialize) {
1210+
query_block.subquery_iterator->EndPSIBatchModeIfStarted();
1211+
}
1212+
m_table_iterator->EndPSIBatchModeIfStarted();
1213+
}
1214+
12171215
bool MaterializeIterator::doing_deduplication() const {
12181216
if (doing_hash_deduplication()) {
12191217
return true;
@@ -1341,7 +1339,7 @@ bool TemptableAggregateIterator::Init() {
13411339
auto end_unique_index =
13421340
create_scope_guard([&] { table()->file->ha_index_end(); });
13431341

1344-
PFSBatchMode pfs_batch_mode(nullptr, m_join);
1342+
PFSBatchMode pfs_batch_mode(m_subquery_iterator.get());
13451343
for (;;) {
13461344
int error = m_subquery_iterator->Read();
13471345
if (error > 0 || thd()->is_error()) // Fatal error
@@ -1996,6 +1994,7 @@ AppendIterator::AppendIterator(
19961994

19971995
bool AppendIterator::Init() {
19981996
m_current_iterator_index = 0;
1997+
m_pfs_batch_mode_enabled = false;
19991998
return m_sub_iterators[0]->Init();
20001999
}
20012000

@@ -2011,12 +2010,16 @@ int AppendIterator::Read() {
20112010
}
20122011

20132012
// EOF. Go to the next iterator.
2013+
m_sub_iterators[m_current_iterator_index]->EndPSIBatchModeIfStarted();
20142014
if (++m_current_iterator_index >= m_sub_iterators.size()) {
20152015
return -1;
20162016
}
20172017
if (m_sub_iterators[m_current_iterator_index]->Init()) {
20182018
return 1;
20192019
}
2020+
if (m_pfs_batch_mode_enabled) {
2021+
m_sub_iterators[m_current_iterator_index]->StartPSIBatchMode();
2022+
}
20202023
return Read(); // Try again, with the new iterator as current.
20212024
}
20222025

@@ -2033,6 +2036,19 @@ void AppendIterator::SetNullRowFlag(bool is_null_row) {
20332036
m_sub_iterators[m_current_iterator_index]->SetNullRowFlag(is_null_row);
20342037
}
20352038

2039+
void AppendIterator::StartPSIBatchMode() {
2040+
m_pfs_batch_mode_enabled = true;
2041+
m_sub_iterators[m_current_iterator_index]->StartPSIBatchMode();
2042+
}
2043+
2044+
void AppendIterator::EndPSIBatchModeIfStarted() {
2045+
for (const unique_ptr_destroy_only<RowIterator> &sub_iterator :
2046+
m_sub_iterators) {
2047+
sub_iterator->EndPSIBatchModeIfStarted();
2048+
}
2049+
m_pfs_batch_mode_enabled = false;
2050+
}
2051+
20362052
void AppendIterator::UnlockRow() {
20372053
DBUG_ASSERT(m_current_iterator_index < m_sub_iterators.size());
20382054
m_sub_iterators[m_current_iterator_index]->UnlockRow();

Diff for: ‎sql/composite_iterators.h

+81-2
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ class FilterIterator final : public RowIterator {
8080
m_source->SetNullRowFlag(is_null_row);
8181
}
8282

83+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
84+
void EndPSIBatchModeIfStarted() override {
85+
m_source->EndPSIBatchModeIfStarted();
86+
}
8387
void UnlockRow() override { m_source->UnlockRow(); }
8488

8589
std::vector<Child> children() const override;
@@ -133,6 +137,10 @@ class LimitOffsetIterator final : public RowIterator {
133137
m_source->SetNullRowFlag(is_null_row);
134138
}
135139

140+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
141+
void EndPSIBatchModeIfStarted() override {
142+
m_source->EndPSIBatchModeIfStarted();
143+
}
136144
void UnlockRow() override { m_source->UnlockRow(); }
137145

138146
std::vector<Child> children() const override {
@@ -217,7 +225,17 @@ class AggregateIterator final : public RowIterator {
217225
void SetNullRowFlag(bool is_null_row) override {
218226
m_source->SetNullRowFlag(is_null_row);
219227
}
220-
void UnlockRow() override;
228+
229+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
230+
void EndPSIBatchModeIfStarted() override {
231+
m_source->EndPSIBatchModeIfStarted();
232+
}
233+
void UnlockRow() override {
234+
// Most likely, HAVING failed. Ideally, we'd like to backtrack and
235+
// unlock all rows that went into this aggregate, but we can't do that,
236+
// and we also can't unlock the _current_ row, since that belongs to a
237+
// different group. Thus, do nothing.
238+
}
221239

222240
std::vector<Child> children() const override {
223241
return std::vector<Child>{{m_source.get(), ""}};
@@ -351,7 +369,14 @@ class PrecomputedAggregateIterator final : public RowIterator {
351369
void SetNullRowFlag(bool is_null_row) override {
352370
m_source->SetNullRowFlag(is_null_row);
353371
}
354-
void UnlockRow() override;
372+
373+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
374+
void EndPSIBatchModeIfStarted() override {
375+
m_source->EndPSIBatchModeIfStarted();
376+
}
377+
void UnlockRow() override {
378+
// See AggregateIterator::UnlockRow().
379+
}
355380

356381
std::vector<Child> children() const override {
357382
return std::vector<Child>{{m_source.get(), ""}};
@@ -424,6 +449,11 @@ class NestedLoopIterator final : public RowIterator {
424449
m_source_inner->SetNullRowFlag(is_null_row);
425450
}
426451

452+
void EndPSIBatchModeIfStarted() override {
453+
m_source_outer->EndPSIBatchModeIfStarted();
454+
m_source_inner->EndPSIBatchModeIfStarted();
455+
}
456+
427457
void UnlockRow() override {
428458
// Since we don't know which condition that caused the row to be rejected,
429459
// we can't know whether we could also unlock the outer row
@@ -666,6 +696,9 @@ class MaterializeIterator final : public TableRowIterator {
666696
m_table_iterator->SetNullRowFlag(is_null_row);
667697
}
668698

699+
void StartPSIBatchMode() override { m_table_iterator->StartPSIBatchMode(); }
700+
void EndPSIBatchModeIfStarted() override;
701+
669702
// The temporary table is private to us, so there's no need to worry about
670703
// locks to other transactions.
671704
void UnlockRow() override {}
@@ -772,6 +805,12 @@ class StreamingIterator final : public TableRowIterator {
772805
return std::vector<Child>{{m_subquery_iterator.get(), ""}};
773806
}
774807

808+
void StartPSIBatchMode() override {
809+
m_subquery_iterator->StartPSIBatchMode();
810+
}
811+
void EndPSIBatchModeIfStarted() override {
812+
m_subquery_iterator->EndPSIBatchModeIfStarted();
813+
}
775814
void UnlockRow() override { m_subquery_iterator->UnlockRow(); }
776815

777816
private:
@@ -799,6 +838,10 @@ class TemptableAggregateIterator final : public TableRowIterator {
799838
void SetNullRowFlag(bool is_null_row) override {
800839
m_table_iterator->SetNullRowFlag(is_null_row);
801840
}
841+
void EndPSIBatchModeIfStarted() override {
842+
m_table_iterator->EndPSIBatchModeIfStarted();
843+
m_subquery_iterator->EndPSIBatchModeIfStarted();
844+
}
802845
void UnlockRow() override {}
803846
std::vector<std::string> DebugString() const override;
804847

@@ -845,6 +888,11 @@ class MaterializedTableFunctionIterator final : public TableRowIterator {
845888
m_table_iterator->SetNullRowFlag(is_null_row);
846889
}
847890

891+
void StartPSIBatchMode() override { m_table_iterator->StartPSIBatchMode(); }
892+
void EndPSIBatchModeIfStarted() override {
893+
m_table_iterator->EndPSIBatchModeIfStarted();
894+
}
895+
848896
// The temporary table is private to us, so there's no need to worry about
849897
// locks to other transactions.
850898
void UnlockRow() override {}
@@ -891,6 +939,9 @@ class WeedoutIterator final : public RowIterator {
891939
m_source->SetNullRowFlag(is_null_row);
892940
}
893941

942+
void EndPSIBatchModeIfStarted() override {
943+
m_source->EndPSIBatchModeIfStarted();
944+
}
894945
void UnlockRow() override { m_source->UnlockRow(); }
895946

896947
private:
@@ -925,6 +976,10 @@ class RemoveDuplicatesIterator final : public RowIterator {
925976
m_source->SetNullRowFlag(is_null_row);
926977
}
927978

979+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
980+
void EndPSIBatchModeIfStarted() override {
981+
m_source->EndPSIBatchModeIfStarted();
982+
}
928983
void UnlockRow() override { m_source->UnlockRow(); }
929984

930985
private:
@@ -983,6 +1038,11 @@ class NestedLoopSemiJoinWithDuplicateRemovalIterator final
9831038
m_source_inner->SetNullRowFlag(is_null_row);
9841039
}
9851040

1041+
void EndPSIBatchModeIfStarted() override {
1042+
m_source_outer->EndPSIBatchModeIfStarted();
1043+
m_source_inner->EndPSIBatchModeIfStarted();
1044+
}
1045+
9861046
void UnlockRow() override {
9871047
m_source_outer->UnlockRow();
9881048
m_source_inner->UnlockRow();
@@ -1030,6 +1090,11 @@ class WindowingIterator final : public RowIterator {
10301090
m_source->SetNullRowFlag(is_null_row);
10311091
}
10321092

1093+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
1094+
void EndPSIBatchModeIfStarted() override {
1095+
m_source->EndPSIBatchModeIfStarted();
1096+
}
1097+
10331098
void UnlockRow() override {
10341099
// There's nothing we can do here.
10351100
}
@@ -1079,6 +1144,11 @@ class BufferingWindowingIterator final : public RowIterator {
10791144
m_source->SetNullRowFlag(is_null_row);
10801145
}
10811146

1147+
void StartPSIBatchMode() override { m_source->StartPSIBatchMode(); }
1148+
void EndPSIBatchModeIfStarted() override {
1149+
m_source->EndPSIBatchModeIfStarted();
1150+
}
1151+
10821152
void UnlockRow() override {
10831153
// There's nothing we can do here.
10841154
}
@@ -1148,6 +1218,11 @@ class MaterializeInformationSchemaTableIterator final : public RowIterator {
11481218
m_table_iterator->SetNullRowFlag(is_null_row);
11491219
}
11501220

1221+
void StartPSIBatchMode() override { m_table_iterator->StartPSIBatchMode(); }
1222+
void EndPSIBatchModeIfStarted() override {
1223+
m_table_iterator->EndPSIBatchModeIfStarted();
1224+
}
1225+
11511226
// The temporary table is private to us, so there's no need to worry about
11521227
// locks to other transactions.
11531228
void UnlockRow() override {}
@@ -1175,12 +1250,16 @@ class AppendIterator final : public RowIterator {
11751250
std::vector<std::string> DebugString() const override { return {"Append"}; }
11761251
std::vector<Child> children() const override;
11771252

1253+
void StartPSIBatchMode() override;
1254+
void EndPSIBatchModeIfStarted() override;
1255+
11781256
void SetNullRowFlag(bool is_null_row) override;
11791257
void UnlockRow() override;
11801258

11811259
private:
11821260
std::vector<unique_ptr_destroy_only<RowIterator>> m_sub_iterators;
11831261
size_t m_current_iterator_index = 0;
1262+
bool m_pfs_batch_mode_enabled = false;
11841263
};
11851264

11861265
#endif // SQL_COMPOSITE_ITERATORS_INCLUDED

Diff for: ‎sql/filesort.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -1061,7 +1061,7 @@ static ha_rows read_all_rows(
10611061
fs_info->clear_peak_memory_used();
10621062
}
10631063

1064-
PFSBatchMode pfs_batch_mode(qep_tab, /*join=*/nullptr);
1064+
source_iterator->StartPSIBatchMode();
10651065
for (;;) {
10661066
DBUG_EXECUTE_IF("bug19656296", DBUG_SET("+d,ha_rnd_next_deadlock"););
10671067
if ((error = source_iterator->Read())) {
@@ -1142,6 +1142,8 @@ static ha_rows read_all_rows(
11421142
}
11431143

11441144
cleanup:
1145+
source_iterator->EndPSIBatchModeIfStarted();
1146+
11451147
// Clear tmp_set so it can be used elsewhere
11461148
bitmap_clear_all(&sort_form->tmp_set);
11471149

0 commit comments

Comments
 (0)