Skip to content

Commit 7aec0ea

Browse files
committed
Pushed dominant db validation into the query analysis stage
MAde the validator a private method of the QuerySession class. Eliminated all other calls to the validator. Adjusted unit tests to skip the validatons since this operation requires non-trivial CSS.
1 parent b9a51ac commit 7aec0ea

3 files changed

Lines changed: 30 additions & 37 deletions

File tree

src/ccontrol/UserQuerySelect.cc

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@
3333
*
3434
* getRestrictors() -- retrieve restrictors to be passed to spatial region selection code in another layer.
3535
*
36-
* validateDominantDbs() -- validate the "dominantDbs", that is, the databases whose
37-
* partitioning will be used for chunking and dispatch. Note that the dominantDbs are required to have
38-
* the same striping parameters to allow joins between them, so it is not sufficient to just validate
39-
* that they are present in the CSS, but they must also have compatible striping parameters.
40-
*
4136
* getDbStriping() -- retrieve the striping parameters of the dominantDb.
4237
*
4338
* getError() -- See if there are errors
@@ -429,13 +424,7 @@ void UserQuerySelect::saveResultQuery() { _queryMetadata->saveResultQuery(_query
429424

430425
void UserQuerySelect::_setupChunking() {
431426
LOGS(_log, LOG_LVL_TRACE, "Setup chunking");
432-
// Do not throw exceptions here, set _errorExtra .
433427
std::shared_ptr<qproc::IndexMap> im;
434-
if (!_qSession->validateDominantDbs()) {
435-
// TODO: Revisit this for L3
436-
throw UserQueryError(getQueryIdString() + " Couldn't determine dominantDbs for dispatch");
437-
}
438-
439428
std::shared_ptr<IntSet const> eSet = _qSession->getEmptyChunks();
440429
{
441430
eSet = _qSession->getEmptyChunks();

src/qproc/QuerySession.cc

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ void QuerySession::analyzeQuery(std::string const& sql, std::shared_ptr<query::S
141141
LOGS(_log, LOG_LVL_TRACE, "Query Plugins applied: " << *this);
142142
LOGS(_log, LOG_LVL_TRACE, "ORDER BY clause for result query: " << getResultOrderBy());
143143

144+
// Besides analysing an explicit result of the operation, allow catching CSS exceptions
145+
// that may be thrown by the method.
146+
if (!_validateDominantDbs()) {
147+
_error = "Dominant databases validation failed";
148+
}
144149
} catch (QueryProcessingBug& b) {
145150
_error = std::string("QuerySession bug:") + b.what();
146151
} catch (qana::AnalysisError& e) {
@@ -216,7 +221,13 @@ bool QuerySession::containsTable(std::string const& dbName, std::string const& t
216221
return _context->containsTable(dbName, tableName);
217222
}
218223

219-
bool QuerySession::validateDominantDbs() const {
224+
bool QuerySession::_validateDominantDbs() const {
225+
if (_skipDominatDbsValidation) {
226+
LOGS(_log, LOG_LVL_DEBUG,
227+
"QuerySession::" << __func__ << ": Skipping dominant DB validation in unit test");
228+
return true;
229+
}
230+
220231
if (_context->dominantDbs.empty()) {
221232
LOGS(_log, LOG_LVL_WARN, "QuerySession::" << __func__ << ": no dominant dbs specified");
222233
return false;
@@ -245,21 +256,13 @@ bool QuerySession::validateDominantDbs() const {
245256
return true;
246257
}
247258

248-
css::StripingParams QuerySession::getDbStriping() const {
249-
if (!validateDominantDbs()) {
250-
throw std::logic_error("QuerySession::" + std::string(__func__) + ": Invalid dominant databases");
251-
}
252-
return _context->getDbStriping();
253-
}
259+
css::StripingParams QuerySession::getDbStriping() const { return _context->getDbStriping(); }
254260

255261
std::shared_ptr<IntSet const> QuerySession::getEmptyChunks() {
256262
if (_css == nullptr) {
257263
LOGS(_log, LOG_LVL_WARN, "QuerySession::" << __func__ << " no _css");
258264
return nullptr;
259265
}
260-
if (!validateDominantDbs()) {
261-
throw std::logic_error("QuerySession::" + std::string(__func__) + ": Invalid dominant databases");
262-
}
263266
std::shared_ptr<IntSet> result = std::make_shared<IntSet>();
264267
for (auto const& dbName : _context->dominantDbs) {
265268
LOGS(_log, LOG_LVL_TRACE, "QuerySession::" << __func__ << " " << dbName);
@@ -298,7 +301,7 @@ void QuerySession::finalize() {
298301
}
299302
}
300303

301-
QuerySession::QuerySession(Test& t) : _css(t.css), _defaultDb(t.defaultDb) {
304+
QuerySession::QuerySession(Test& t) : _css(t.css), _defaultDb(t.defaultDb), _skipDominatDbsValidation(true) {
302305
sql::SqlConfig sqlConfig(t.sqlConfig);
303306
_databaseModels = DatabaseModels::create(sqlConfig, sqlConfig);
304307
_initContext();
@@ -442,10 +445,6 @@ std::ostream& operator<<(std::ostream& out, QuerySession const& querySession) {
442445
ChunkQuerySpec::Ptr QuerySession::buildChunkQuerySpec(query::QueryTemplate::Vect const& queryTemplates,
443446
ChunkSpec const& chunkSpec,
444447
bool fillInChunkIdTag) const {
445-
if (!validateDominantDbs()) {
446-
throw std::logic_error("QuerySession::" + std::string(__func__) + ": Invalid dominant databases");
447-
}
448-
449448
// Any dominant database (if there is more than one) works here. Note that after the previously made
450449
// validation step, there should be at least one such database, and if there are more than one, then
451450
// all databases ara guaranteeded to have the same striping parameters. The only role if the database

src/qproc/QuerySession.h

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,17 +132,6 @@ class QuerySession {
132132
bool containsDb(std::string const& dbName) const;
133133
bool containsTable(std::string const& dbName, std::string const& tableName) const;
134134

135-
/**
136-
* Validate that the dominant databases specified in the query context are registered
137-
* in CSS and compatible with each other.
138-
* @note Dominant databases are the databases that will be used for query
139-
* dispatch. They are distinct from the default database, which is what is
140-
* used for unqualified table and column references.
141-
* @return true if the dominant databases are valid, false otherwise. If false is returned,
142-
* details will be logged.
143-
*/
144-
bool validateDominantDbs() const;
145-
146135
/**
147136
* Get the striping parameters of any dominant database found in the query.
148137
* @note That all dominant databases are required to have the same striping parameters, so it does
@@ -217,6 +206,17 @@ class QuerySession {
217206
void _generateConcrete();
218207
void _applyConcretePlugins();
219208

209+
/**
210+
* Validate that the dominant databases specified in the query context are registered
211+
* in CSS and compatible with each other.
212+
* @note Dominant databases are the databases that will be used for query
213+
* dispatch. They are distinct from the default database, which is what is
214+
* used for unqualified table and column references.
215+
* @return true if the dominant databases are valid, false otherwise. If false is returned,
216+
* details will be logged.
217+
*/
218+
bool _validateDominantDbs() const;
219+
220220
std::vector<std::string> _buildChunkQueries(query::QueryTemplate::Vect const& queryTemplates,
221221
ChunkSpec const& chunkSpec) const;
222222
std::shared_ptr<ChunkQuerySpec> _buildFragment(query::QueryTemplate::Vect const& queryTemplates,
@@ -280,6 +280,11 @@ class QuerySession {
280280
/// Maximum number of chunks in an interactive query.
281281
int const _interactiveChunkLimit = 10; // Value of 10 only used in unit tests.
282282
bool _scanInteractive = true; ///< True if the query can be considered interactive.
283+
284+
/// Whether to validate the dominant databases against CSS. This is set true for unit tests
285+
/// that construct QuerySessions without CSS access, and thus can't validate the dominant
286+
/// databases, and false for all other use cases.
287+
bool const _skipDominatDbsValidation = false;
283288
};
284289

285290
/**

0 commit comments

Comments
 (0)