Skip to content

Commit 6e5aa64

Browse files
committed
Bug#33364732: Preliminary fixes for WL#14673 [force_sort_rowids]
Rename Filesort::m_force_sort_positions to m_force_sort_rowids. “sort_positions” is a name that is left over after some old filesort code, and it's not that descriptive. Change-Id: I369eb34bc436aca66521c3cb3a4a601425f0536c
1 parent 36215a8 commit 6e5aa64

8 files changed

+24
-24
lines changed

sql/filesort.cc

+7-7
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ static bool check_if_pq_applicable(Opt_trace_context *trace, Sort_param *param,
160160

161161
void Sort_param::decide_addon_fields(Filesort *file_sort,
162162
const Mem_root_array<TABLE *> &tables,
163-
bool force_sort_positions) {
163+
bool force_sort_rowids) {
164164
if (m_addon_fields_status != Addon_fields_status::unknown_status) {
165165
// Already decided.
166166
return;
@@ -194,7 +194,7 @@ void Sort_param::decide_addon_fields(Filesort *file_sort,
194194
}
195195
}
196196

197-
if (force_sort_positions) {
197+
if (force_sort_rowids) {
198198
m_addon_fields_status = Addon_fields_status::keep_rowid;
199199
} else {
200200
/*
@@ -230,7 +230,7 @@ void Sort_param::init_for_filesort(Filesort *file_sort,
230230

231231
local_sortorder = sf_array;
232232

233-
decide_addon_fields(file_sort, tables, file_sort->m_force_sort_positions);
233+
decide_addon_fields(file_sort, tables, file_sort->m_force_sort_rowids);
234234
if (using_addon_fields()) {
235235
fixed_res_length = m_addon_length;
236236
} else {
@@ -357,7 +357,7 @@ static void trace_filesort_information(Opt_trace_context *trace,
357357
applying WHERE condition.
358358
359359
@note
360-
If we sort by position (like if sort_positions is 1) filesort() will
360+
If we sort row IDs (as opposed to addon fields), filesort() will
361361
call table->prepare_for_position().
362362
363363
@returns False if success, true if error
@@ -669,7 +669,7 @@ void filesort_free_buffers(TABLE *table, bool full) {
669669

670670
Filesort::Filesort(THD *thd, Mem_root_array<TABLE *> tables_arg,
671671
bool keep_buffers_arg, ORDER *order, ha_rows limit_arg,
672-
bool remove_duplicates, bool sort_positions,
672+
bool remove_duplicates, bool force_sort_rowids,
673673
bool unwrap_rollup)
674674
: m_thd(thd),
675675
tables(std::move(tables_arg)),
@@ -678,7 +678,7 @@ Filesort::Filesort(THD *thd, Mem_root_array<TABLE *> tables_arg,
678678
sortorder(nullptr),
679679
using_pq(false),
680680
m_remove_duplicates(remove_duplicates),
681-
m_force_sort_positions(sort_positions),
681+
m_force_sort_rowids(force_sort_rowids),
682682
m_sort_order_length(make_sortorder(order, unwrap_rollup)) {}
683683

684684
uint Filesort::make_sortorder(ORDER *order, bool unwrap_rollup) {
@@ -2297,7 +2297,7 @@ Addon_fields *Filesort::get_addon_fields(
22972297
bool Filesort::using_addon_fields() {
22982298
if (m_sort_param.m_addon_fields_status ==
22992299
Addon_fields_status::unknown_status) {
2300-
m_sort_param.decide_addon_fields(this, tables, m_force_sort_positions);
2300+
m_sort_param.decide_addon_fields(this, tables, m_force_sort_rowids);
23012301
}
23022302
return m_sort_param.using_addon_fields();
23032303
}

sql/filesort.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ class Filesort {
6666
// If true, we will always sort references to rows on table (and crucially,
6767
// the result iterators used will always position the underlying table on
6868
// the original row before returning from Read()).
69-
bool m_force_sort_positions;
69+
bool m_force_sort_rowids;
7070
// TODO: Consider moving this into private members of Filesort.
7171
Sort_param m_sort_param;
7272

@@ -75,7 +75,7 @@ class Filesort {
7575
// so we can't do that yet.
7676
Filesort(THD *thd, Mem_root_array<TABLE *> tables, bool keep_buffers,
7777
ORDER *order, ha_rows limit_arg, bool remove_duplicates,
78-
bool force_sort_positions, bool unwrap_rollup);
78+
bool force_sort_rowids, bool unwrap_rollup);
7979

8080
Addon_fields *get_addon_fields(Addon_fields_status *addon_fields_status,
8181
uint *plength, uint *ppackable_length);

sql/sort_param.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ class Sort_param {
330330
/// precise estimation of packed row size.
331331
void decide_addon_fields(Filesort *file_sort,
332332
const Mem_root_array<TABLE *> &tables,
333-
bool sort_positions);
333+
bool force_sort_rowids);
334334

335335
/// Reset the decision made in decide_addon_fields(). Only used in exceptional
336336
/// circumstances (see NewWeedoutAccessPathForTables()).

sql/sql_delete.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ bool Sql_cmd_delete::delete_from_single_table(THD *thd) {
530530
fsort.reset(new (thd->mem_root) Filesort(
531531
thd, {table}, /*keep_buffers=*/false, order, HA_POS_ERROR,
532532
/*remove_duplicates=*/false,
533-
/*force_sort_positions=*/true, /*unwrap_rollup=*/false));
533+
/*force_sort_rowids=*/true, /*unwrap_rollup=*/false));
534534
path = NewSortAccessPath(thd, path, fsort.get(),
535535
/*count_examined_rows=*/false);
536536
iterator = CreateIteratorFromAccessPath(thd, path, &join,

sql/sql_executor.cc

+8-8
Original file line numberDiff line numberDiff line change
@@ -1187,7 +1187,7 @@ static AccessPath *NewWeedoutAccessPathForTables(
11871187
// the previous (now wrong) decision there.
11881188
filesort->clear_addon_fields();
11891189
}
1190-
filesort->m_force_sort_positions = true;
1190+
filesort->m_force_sort_rowids = true;
11911191
}
11921192
}
11931193
}
@@ -3029,7 +3029,7 @@ AccessPath *JOIN::create_root_access_path_for_join() {
30293029
// Only const fields.
30303030
limit_1_for_dup_filesort = true;
30313031
} else {
3032-
bool force_sort_positions = false;
3032+
bool force_sort_rowids = false;
30333033
if (all_order_fields_used) {
30343034
// The ordering for DISTINCT already gave us the right sort order,
30353035
// so no need to sort again.
@@ -3043,27 +3043,27 @@ AccessPath *JOIN::create_root_access_path_for_join() {
30433043
} else if (filesort != nullptr && !filesort->using_addon_fields()) {
30443044
// We have the rather unusual situation here that we have two sorts
30453045
// directly after each other, with no temporary table in-between,
3046-
// and filesort expects to be able to refer to rows by their position.
3046+
// and filesort expects to be able to refer to rows by their row ID.
30473047
// Usually, the sort for DISTINCT would be a superset of the sort for
30483048
// ORDER BY, but not always (e.g. when sorting by some expression),
30493049
// so we could end up in a situation where the first sort is by addon
30503050
// fields and the second one is by positions.
30513051
//
3052-
// Thus, in this case, we force the first sort to be by positions,
3052+
// Thus, in this case, we force the first sort to use row IDs,
30533053
// so that the result comes from SortFileIndirectIterator or
30543054
// SortBufferIndirectIterator. These will both position the cursor
30553055
// on the underlying temporary table correctly before returning it,
3056-
// so that the successive filesort will save the right position
3056+
// so that the successive filesort will save the right row ID
30573057
// for the row.
3058-
force_sort_positions = true;
3058+
force_sort_rowids = true;
30593059
}
30603060

30613061
// Switch to the right slice if applicable, so that we fetch out the
30623062
// correct items from order_arg.
30633063
Switch_ref_item_slice slice_switch(this, qep_tab->ref_item_slice);
30643064
dup_filesort = new (thd->mem_root) Filesort(
30653065
thd, {qep_tab->table()}, /*keep_buffers=*/false, order,
3066-
HA_POS_ERROR, /*remove_duplicates=*/true, force_sort_positions,
3066+
HA_POS_ERROR, /*remove_duplicates=*/true, force_sort_rowids,
30673067
/*unwrap_rollup=*/false);
30683068

30693069
if (desired_order != nullptr && filesort == nullptr) {
@@ -3072,7 +3072,7 @@ AccessPath *JOIN::create_root_access_path_for_join() {
30723072
// potentially addon fields. Create a new one.
30733073
filesort = new (thd->mem_root) Filesort(
30743074
thd, {qep_tab->table()}, /*keep_buffers=*/false, desired_order,
3075-
HA_POS_ERROR, /*remove_duplicates=*/false, force_sort_positions,
3075+
HA_POS_ERROR, /*remove_duplicates=*/false, force_sort_rowids,
30763076
/*unwrap_rollup=*/false);
30773077
}
30783078
}

sql/sql_select.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -4815,7 +4815,7 @@ bool JOIN::add_sorting_to_table(uint idx, ORDER_with_src *sort_order,
48154815
// by row ID), so if this table is part of a weedout operation, we need
48164816
// to force sorting by row IDs -- sorting rows with addon fields returns
48174817
// rows that have no reference to the underlying table object.
4818-
bool force_sort_position = false;
4818+
bool force_sort_rowids = false;
48194819
for (plan_idx i = 0; i <= static_cast<plan_idx>(idx); ++i) {
48204820
if (!qep_tab[i].starts_weedout()) {
48214821
continue;
@@ -4830,7 +4830,7 @@ bool JOIN::add_sorting_to_table(uint idx, ORDER_with_src *sort_order,
48304830
}
48314831
if (weedout_end != NO_PLAN_IDX &&
48324832
weedout_end > static_cast<plan_idx>(idx)) {
4833-
force_sort_position = true;
4833+
force_sort_rowids = true;
48344834
break;
48354835
}
48364836
}
@@ -4851,7 +4851,7 @@ bool JOIN::add_sorting_to_table(uint idx, ORDER_with_src *sort_order,
48514851
Switch_ref_item_slice slice_switch(this, tab->ref_item_slice);
48524852
tab->filesort = new (thd->mem_root)
48534853
Filesort(thd, {tab->table()}, keep_buffers, sort_order->order,
4854-
HA_POS_ERROR, /*remove_duplicates=*/false, force_sort_position,
4854+
HA_POS_ERROR, /*remove_duplicates=*/false, force_sort_rowids,
48554855
/*unwrap_rollup=*/sort_before_group);
48564856
tab->filesort_pushed_order = sort_order->order;
48574857
}

sql/sql_table.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -18114,7 +18114,7 @@ static int copy_data_between_tables(
1811418114
}
1811518115
fsort.reset(new (thd->mem_root) Filesort(
1811618116
thd, {from}, /*keep_buffers=*/false, order, HA_POS_ERROR,
18117-
/*remove_duplicates=*/false, /*force_sort_positions=*/true,
18117+
/*remove_duplicates=*/false, /*force_sort_rowids=*/true,
1811818118
/*unwrap_rollup=*/false));
1811918119
path = NewSortAccessPath(thd, path, fsort.get(),
1812018120
/*count_examined_rows=*/false);

sql/sql_update.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ bool Sql_cmd_update::update_single_table(THD *thd) {
664664
fsort.reset(new (thd->mem_root) Filesort(
665665
thd, {table}, /*keep_buffers=*/false, order, limit,
666666
/*remove_duplicates=*/false,
667-
/*force_sort_positions=*/true, /*unwrap_rollup=*/false));
667+
/*force_sort_rowids=*/true, /*unwrap_rollup=*/false));
668668
path = NewSortAccessPath(thd, path, fsort.get(),
669669
/*count_examined_rows=*/false);
670670
iterator = CreateIteratorFromAccessPath(

0 commit comments

Comments
 (0)