Skip to content

Commit d227c22

Browse files
authored
Move common strategy parameters to class. (#4620)
This moves common strategy parameters to a class so that adding/updating parameters is easier moving forward. Note that this doesn't move the stats and logger as those get modified in many places down the construction chain so that will require a later change that can be done with more care. --- TYPE: NO_HISTORY DESC: Move common strategy parameters to class.
1 parent 6b29117 commit d227c22

28 files changed

+267
-577
lines changed

test/src/unit-Reader.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,7 @@ TEST_CASE_METHOD(
163163
.ok());
164164
Subarray subarray(&array, &g_helper_stats, g_helper_logger());
165165
DefaultChannelAggregates default_channel_aggregates;
166-
Reader reader(
167-
&g_helper_stats,
168-
g_helper_logger(),
166+
auto params = StrategyParams(
169167
context.storage_manager(),
170168
&array,
171169
config,
@@ -174,7 +172,9 @@ TEST_CASE_METHOD(
174172
subarray,
175173
Layout::ROW_MAJOR,
176174
condition,
177-
default_channel_aggregates);
175+
default_channel_aggregates,
176+
false);
177+
Reader reader(&g_helper_stats, g_helper_logger(), params);
178178
unsigned dim_num = 2;
179179
auto size = 2 * sizeof(int32_t);
180180
int32_t domain_vec[] = {1, 10, 1, 15};

tiledb/sm/query/deletes_and_updates/deletes_and_updates.cc

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,25 +57,10 @@ class DeleteAndUpdateStatusException : public StatusException {
5757
DeletesAndUpdates::DeletesAndUpdates(
5858
stats::Stats* stats,
5959
shared_ptr<Logger> logger,
60-
StorageManager* storage_manager,
61-
Array* array,
62-
Config& config,
63-
std::unordered_map<std::string, QueryBuffer>& buffers,
64-
Subarray& subarray,
65-
Layout layout,
66-
std::optional<QueryCondition>& condition,
67-
std::vector<UpdateValue>& update_values,
68-
bool skip_checks_serialization)
69-
: StrategyBase(
70-
stats,
71-
logger->clone("Deletes", ++logger_id_),
72-
storage_manager,
73-
array,
74-
config,
75-
buffers,
76-
subarray,
77-
layout)
78-
, condition_(condition)
60+
StrategyParams& params,
61+
std::vector<UpdateValue>& update_values)
62+
: StrategyBase(stats, logger->clone("Deletes", ++logger_id_), params)
63+
, condition_(params.condition())
7964
, update_values_(update_values) {
8065
// Sanity checks
8166
if (storage_manager_ == nullptr) {
@@ -98,7 +83,7 @@ DeletesAndUpdates::DeletesAndUpdates(
9883
"Cannot initialize deletes; Subarrays are not supported");
9984
}
10085

101-
if (!skip_checks_serialization && !condition_.has_value()) {
86+
if (!params.skip_checks_serialization() && !condition_.has_value()) {
10287
throw DeleteAndUpdateStatusException(
10388
"Cannot initialize deletes; One condition is needed");
10489
}

tiledb/sm/query/deletes_and_updates/deletes_and_updates.h

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,8 @@ class DeletesAndUpdates : public StrategyBase, public IQueryStrategy {
5454
DeletesAndUpdates(
5555
stats::Stats* stats,
5656
shared_ptr<Logger> logger,
57-
StorageManager* storage_manager,
58-
Array* array,
59-
Config& config,
60-
std::unordered_map<std::string, QueryBuffer>& buffers,
61-
Subarray& subarray,
62-
Layout layout,
63-
std::optional<QueryCondition>& condition,
64-
std::vector<UpdateValue>& update_values,
65-
bool skip_checks_serialization = false);
57+
StrategyParams& params,
58+
std::vector<UpdateValue>& update_values);
6659

6760
/** Destructor. */
6861
~DeletesAndUpdates();

tiledb/sm/query/legacy/reader.cc

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -107,52 +107,32 @@ inline IterT skip_invalid_elements(IterT it, const IterT& end) {
107107
Reader::Reader(
108108
stats::Stats* stats,
109109
shared_ptr<Logger> logger,
110-
StorageManager* storage_manager,
111-
Array* array,
112-
Config& config,
113-
std::unordered_map<std::string, QueryBuffer>& buffers,
114-
std::unordered_map<std::string, QueryBuffer>& aggregate_buffers,
115-
Subarray& subarray,
116-
Layout layout,
117-
std::optional<QueryCondition>& condition,
118-
DefaultChannelAggregates& default_channel_aggregates,
119-
bool skip_checks_serialization,
110+
StrategyParams& params,
120111
bool remote_query)
121-
: ReaderBase(
122-
stats,
123-
logger->clone("Reader", ++logger_id_),
124-
storage_manager,
125-
array,
126-
config,
127-
buffers,
128-
aggregate_buffers,
129-
subarray,
130-
layout,
131-
condition,
132-
default_channel_aggregates) {
112+
: ReaderBase(stats, logger->clone("Reader", ++logger_id_), params) {
133113
// Sanity checks
134114
if (storage_manager_ == nullptr) {
135115
throw ReaderStatusException(
136116
"Cannot initialize reader; Storage manager not set");
137117
}
138118

139-
if (!default_channel_aggregates.empty()) {
119+
if (!params.default_channel_aggregates().empty()) {
140120
throw ReaderStatusException(
141121
"Cannot initialize reader; Reader cannot process aggregates");
142122
}
143123

144-
if (!skip_checks_serialization && buffers_.empty()) {
124+
if (!params.skip_checks_serialization() && buffers_.empty()) {
145125
throw ReaderStatusException("Cannot initialize reader; Buffers not set");
146126
}
147127

148-
if (!skip_checks_serialization && array_schema_.dense() &&
128+
if (!params.skip_checks_serialization() && array_schema_.dense() &&
149129
!subarray_.is_set()) {
150130
throw ReaderStatusException(
151131
"Cannot initialize reader; Dense reads must have a subarray set");
152132
}
153133

154134
// Check subarray
155-
check_subarray(remote_query && array->array_schema_latest().dense());
135+
check_subarray(remote_query && params.array()->array_schema_latest().dense());
156136

157137
// Initialize the read state
158138
init_read_state();

tiledb/sm/query/legacy/reader.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,7 @@ class Reader : public ReaderBase, public IQueryStrategy {
6767
Reader(
6868
stats::Stats* stats,
6969
shared_ptr<Logger> logger,
70-
StorageManager* storage_manager,
71-
Array* array,
72-
Config& config,
73-
std::unordered_map<std::string, QueryBuffer>& buffers,
74-
std::unordered_map<std::string, QueryBuffer>& aggregate_buffers,
75-
Subarray& subarray,
76-
Layout layout,
77-
std::optional<QueryCondition>& condition,
78-
DefaultChannelAggregates& default_channel_aggregates,
79-
bool skip_checks_serialization = false,
70+
StrategyParams& params,
8071
bool remote_query = false);
8172

8273
/** Destructor. */

tiledb/sm/query/query.cc

Lines changed: 29 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,17 @@ bool Query::is_aggregate(std::string output_field_name) const {
17891789
/* ****************************** */
17901790

17911791
Status Query::create_strategy(bool skip_checks_serialization) {
1792+
auto params = StrategyParams(
1793+
storage_manager_,
1794+
array_,
1795+
config_,
1796+
buffers_,
1797+
aggregate_buffers_,
1798+
subarray_,
1799+
layout_,
1800+
condition_,
1801+
default_channel_aggregates_,
1802+
skip_checks_serialization);
17921803
if (type_ == QueryType::WRITE || type_ == QueryType::MODIFY_EXCLUSIVE) {
17931804
if (layout_ == Layout::COL_MAJOR || layout_ == Layout::ROW_MAJOR) {
17941805
if (!array_schema_->dense()) {
@@ -1800,17 +1811,11 @@ Status Query::create_strategy(bool skip_checks_serialization) {
18001811
OrderedWriter,
18011812
stats_->create_child("Writer"),
18021813
logger_,
1803-
storage_manager_,
1804-
array_,
1805-
config_,
1806-
buffers_,
1807-
subarray_,
1808-
layout_,
1814+
params,
18091815
written_fragment_info_,
18101816
coords_info_,
18111817
remote_query_,
1812-
fragment_name_,
1813-
skip_checks_serialization));
1818+
fragment_name_));
18141819
} else if (layout_ == Layout::UNORDERED) {
18151820
if (array_schema_->dense()) {
18161821
return Status_QueryError(
@@ -1821,37 +1826,25 @@ Status Query::create_strategy(bool skip_checks_serialization) {
18211826
UnorderedWriter,
18221827
stats_->create_child("Writer"),
18231828
logger_,
1824-
storage_manager_,
1825-
array_,
1826-
config_,
1827-
buffers_,
1828-
subarray_,
1829-
layout_,
1829+
params,
18301830
written_fragment_info_,
18311831
coords_info_,
18321832
written_buffers_,
18331833
remote_query_,
1834-
fragment_name_,
1835-
skip_checks_serialization));
1834+
fragment_name_));
18361835
} else if (layout_ == Layout::GLOBAL_ORDER) {
18371836
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
18381837
GlobalOrderWriter,
18391838
stats_->create_child("Writer"),
18401839
logger_,
1841-
storage_manager_,
1842-
array_,
1843-
config_,
1844-
buffers_,
1845-
subarray_,
1846-
layout_,
1840+
params,
18471841
fragment_size_,
18481842
written_fragment_info_,
18491843
disable_checks_consolidation_,
18501844
processed_conditions_,
18511845
coords_info_,
18521846
remote_query_,
1853-
fragment_name_,
1854-
skip_checks_serialization));
1847+
fragment_name_));
18551848
} else {
18561849
return Status_QueryError(
18571850
"Cannot create strategy; unsupported layout " + layout_str(layout_));
@@ -1867,17 +1860,8 @@ Status Query::create_strategy(bool skip_checks_serialization) {
18671860
OrderedDimLabelReader,
18681861
stats_->create_child("Reader"),
18691862
logger_,
1870-
storage_manager_,
1871-
array_,
1872-
config_,
1873-
buffers_,
1874-
aggregate_buffers_,
1875-
subarray_,
1876-
layout_,
1877-
condition_,
1878-
default_channel_aggregates_,
1879-
dimension_label_increasing_,
1880-
skip_checks_serialization));
1863+
params,
1864+
dimension_label_increasing_));
18811865
} else if (use_refactored_sparse_unordered_with_dups_reader(
18821866
layout_, *array_schema_)) {
18831867
auto&& [st, non_overlapping_ranges]{Query::non_overlapping_ranges()};
@@ -1889,31 +1873,13 @@ Status Query::create_strategy(bool skip_checks_serialization) {
18891873
SparseUnorderedWithDupsReader<uint8_t>,
18901874
stats_->create_child("Reader"),
18911875
logger_,
1892-
storage_manager_,
1893-
array_,
1894-
config_,
1895-
buffers_,
1896-
aggregate_buffers_,
1897-
subarray_,
1898-
layout_,
1899-
condition_,
1900-
default_channel_aggregates_,
1901-
skip_checks_serialization));
1876+
params));
19021877
} else {
19031878
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
19041879
SparseUnorderedWithDupsReader<uint64_t>,
19051880
stats_->create_child("Reader"),
19061881
logger_,
1907-
storage_manager_,
1908-
array_,
1909-
config_,
1910-
buffers_,
1911-
aggregate_buffers_,
1912-
subarray_,
1913-
layout_,
1914-
condition_,
1915-
default_channel_aggregates_,
1916-
skip_checks_serialization));
1882+
params));
19171883
}
19181884
} else if (
19191885
use_refactored_sparse_global_order_reader(layout_, *array_schema_) &&
@@ -1929,81 +1895,38 @@ Status Query::create_strategy(bool skip_checks_serialization) {
19291895
SparseGlobalOrderReader<uint8_t>,
19301896
stats_->create_child("Reader"),
19311897
logger_,
1932-
storage_manager_,
1933-
array_,
1934-
config_,
1935-
buffers_,
1936-
aggregate_buffers_,
1937-
subarray_,
1938-
layout_,
1939-
condition_,
1940-
default_channel_aggregates_,
1941-
consolidation_with_timestamps_,
1942-
skip_checks_serialization));
1898+
params,
1899+
consolidation_with_timestamps_));
19431900
} else {
19441901
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
19451902
SparseGlobalOrderReader<uint64_t>,
19461903
stats_->create_child("Reader"),
19471904
logger_,
1948-
storage_manager_,
1949-
array_,
1950-
config_,
1951-
buffers_,
1952-
aggregate_buffers_,
1953-
subarray_,
1954-
layout_,
1955-
condition_,
1956-
default_channel_aggregates_,
1957-
consolidation_with_timestamps_,
1958-
skip_checks_serialization));
1905+
params,
1906+
consolidation_with_timestamps_));
19591907
}
19601908
} else if (use_refactored_dense_reader(*array_schema_, all_dense)) {
19611909
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
19621910
DenseReader,
19631911
stats_->create_child("Reader"),
19641912
logger_,
1965-
storage_manager_,
1966-
array_,
1967-
config_,
1968-
buffers_,
1969-
aggregate_buffers_,
1970-
subarray_,
1971-
layout_,
1972-
condition_,
1973-
default_channel_aggregates_,
1974-
skip_checks_serialization,
1913+
params,
19751914
remote_query_));
19761915
} else {
19771916
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
19781917
Reader,
19791918
stats_->create_child("Reader"),
19801919
logger_,
1981-
storage_manager_,
1982-
array_,
1983-
config_,
1984-
buffers_,
1985-
aggregate_buffers_,
1986-
subarray_,
1987-
layout_,
1988-
condition_,
1989-
default_channel_aggregates_,
1990-
skip_checks_serialization,
1920+
params,
19911921
remote_query_));
19921922
}
19931923
} else if (type_ == QueryType::DELETE || type_ == QueryType::UPDATE) {
19941924
strategy_ = tdb_unique_ptr<IQueryStrategy>(tdb_new(
19951925
DeletesAndUpdates,
19961926
stats_->create_child("Deletes"),
19971927
logger_,
1998-
storage_manager_,
1999-
array_,
2000-
config_,
2001-
buffers_,
2002-
subarray_,
2003-
layout_,
2004-
condition_,
2005-
update_values_,
2006-
skip_checks_serialization));
1928+
params,
1929+
update_values_));
20071930
} else {
20081931
return logger_->status(
20091932
Status_QueryError("Cannot create strategy; unsupported query type"));

0 commit comments

Comments
 (0)