From 50d830d924fdee5fc8dc4533b306d54922ead741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20B=C3=B6hme?= Date: Thu, 22 Feb 2018 13:24:00 -0500 Subject: [PATCH] Exit immediately when query is invalid (#97) --- mpi/mpi-caliquery/mpi-caliquery.cpp | 13 ++++- src/tools/cali-query/cali-query.cpp | 9 +++- src/tools/cali-query/query_common.cpp | 75 +++++++++++++++------------ src/tools/cali-query/query_common.h | 23 +++++++- 4 files changed, 82 insertions(+), 38 deletions(-) diff --git a/mpi/mpi-caliquery/mpi-caliquery.cpp b/mpi/mpi-caliquery/mpi-caliquery.cpp index 4df43c4bb..374ea3634 100644 --- a/mpi/mpi-caliquery/mpi-caliquery.cpp +++ b/mpi/mpi-caliquery/mpi-caliquery.cpp @@ -243,7 +243,17 @@ int main(int argc, char* argv[]) MPI_Abort(MPI_COMM_WORLD, -1); } - QuerySpec spec = spec_from_args(args); + QueryArgsParser query_parser; + + if (!query_parser.parse_args(args)) { + if (rank == 0) + std::cerr << "cali-query: Invalid query: " << query_parser.error_msg() + << std::endl; + + MPI_Abort(MPI_COMM_WORLD, -2); + } + + QuerySpec spec = query_parser.spec(); Aggregator aggregate(spec); CaliperMetadataDB metadb; @@ -264,6 +274,7 @@ int main(int argc, char* argv[]) if (rank == 0) ::format_output(args, spec, metadb, aggregate); + MPI_Finalize(); return 0; diff --git a/src/tools/cali-query/cali-query.cpp b/src/tools/cali-query/cali-query.cpp index 93d018b2b..454e0d6d4 100644 --- a/src/tools/cali-query/cali-query.cpp +++ b/src/tools/cali-query/cali-query.cpp @@ -307,7 +307,14 @@ int main(int argc, const char* argv[]) // --- Build up processing chain (from back to front) // - QuerySpec spec = spec_from_args(args); + QueryArgsParser query_parser; + + if (!query_parser.parse_args(args)) { + cerr << "cali-query: Invalid query: " << query_parser.error_msg() << std::endl; + return -2; + } + + QuerySpec spec = query_parser.spec(); // setup format spec diff --git a/src/tools/cali-query/query_common.cpp b/src/tools/cali-query/query_common.cpp index b7c44e5ca..78db4f80e 100644 --- a/src/tools/cali-query/query_common.cpp +++ b/src/tools/cali-query/query_common.cpp @@ -171,17 +171,18 @@ parse_functioncall(std::istream& is, const QuerySpec::FunctionSignature* defs) namespace cali { -QuerySpec -spec_from_args(const Args& args) +bool +QueryArgsParser::parse_args(const Args& args) { - QuerySpec spec; - - spec.filter.selection = QuerySpec::FilterSelection::Default; - spec.attribute_selection.selection = QuerySpec::AttributeSelection::Default; - spec.aggregation_ops.selection = QuerySpec::AggregationSelection::None; - spec.aggregation_key.selection = QuerySpec::AttributeSelection::None; - spec.sort.selection = QuerySpec::SortSelection::Default; - spec.format.opt = QuerySpec::FormatSpec::Default; + m_spec.filter.selection = QuerySpec::FilterSelection::Default; + m_spec.attribute_selection.selection = QuerySpec::AttributeSelection::Default; + m_spec.aggregation_ops.selection = QuerySpec::AggregationSelection::None; + m_spec.aggregation_key.selection = QuerySpec::AttributeSelection::None; + m_spec.sort.selection = QuerySpec::SortSelection::Default; + m_spec.format.opt = QuerySpec::FormatSpec::Default; + + m_error = false; + m_error_msg.clear(); // parse CalQL query (if any) @@ -189,36 +190,38 @@ spec_from_args(const Args& args) std::string q = args.get("query"); CalQLParser p(q.c_str()); - if (p.error()) - Log(0).stream() << "Query parse error: " << q << ": " << p.error_msg(); - else - spec = p.spec(); + if (p.error()) { + m_error = true; + m_error_msg = p.error_msg(); + return false; + } else + m_spec = p.spec(); } // setup filter if (args.is_set("select")) { - spec.filter.selection = QuerySpec::FilterSelection::List; - spec.filter.list = RecordSelector::parse(args.get("select")); + m_spec.filter.selection = QuerySpec::FilterSelection::List; + m_spec.filter.list = RecordSelector::parse(args.get("select")); } // setup attribute selection if (args.is_set("attributes")) { - spec.attribute_selection.selection = QuerySpec::AttributeSelection::List; - util::split(args.get("attributes"), ',', std::back_inserter(spec.attribute_selection.list)); + m_spec.attribute_selection.selection = QuerySpec::AttributeSelection::List; + util::split(args.get("attributes"), ',', std::back_inserter(m_spec.attribute_selection.list)); } // setup aggregation if (args.is_set("aggregate")) { // aggregation ops - spec.aggregation_ops.selection = QuerySpec::AggregationSelection::Default; + m_spec.aggregation_ops.selection = QuerySpec::AggregationSelection::Default; std::string opstr = args.get("aggregate"); if (!opstr.empty()) { - spec.aggregation_ops.selection = QuerySpec::AggregationSelection::List; + m_spec.aggregation_ops.selection = QuerySpec::AggregationSelection::List; std::istringstream is(opstr); char c; @@ -229,23 +232,23 @@ spec_from_args(const Args& args) auto fpair = parse_functioncall(is, defs); if (fpair.first >= 0) - spec.aggregation_ops.list.emplace_back(defs[fpair.first], fpair.second, ""); + m_spec.aggregation_ops.list.emplace_back(defs[fpair.first], fpair.second, ""); c = parse_char(is); } while (is.good() && c == ','); } // aggregation key - spec.aggregation_key.selection = QuerySpec::AttributeSelection::Default; + m_spec.aggregation_key.selection = QuerySpec::AttributeSelection::Default; if (args.is_set("aggregate-key")) { std::string keystr = args.get("aggregate-key"); if (keystr == "none") { - spec.aggregation_key.selection = QuerySpec::AttributeSelection::None; + m_spec.aggregation_key.selection = QuerySpec::AttributeSelection::None; } else { - spec.aggregation_key.selection = QuerySpec::AttributeSelection::List; - util::split(keystr, ',', std::back_inserter(spec.aggregation_key.list)); + m_spec.aggregation_key.selection = QuerySpec::AttributeSelection::List; + util::split(keystr, ',', std::back_inserter(m_spec.aggregation_key.list)); } } } @@ -253,13 +256,13 @@ spec_from_args(const Args& args) // setup sort if (args.is_set("sort")) { - spec.sort.selection = QuerySpec::SortSelection::List; + m_spec.sort.selection = QuerySpec::SortSelection::List; std::vector list; util::split(args.get("sort"), ',', std::back_inserter(list)); for (const std::string& s : list) - spec.sort.list.emplace_back(s); + m_spec.sort.list.emplace_back(s); } // setup formatter @@ -267,25 +270,29 @@ spec_from_args(const Args& args) for (const QuerySpec::FunctionSignature* fmtsig = FormatProcessor::formatter_defs(); fmtsig && fmtsig->name; ++fmtsig) { // see if a formatting option is set if (args.is_set(fmtsig->name)) { - spec.format.opt = QuerySpec::FormatSpec::User; - spec.format.formatter = *fmtsig; + m_spec.format.opt = QuerySpec::FormatSpec::User; + m_spec.format.formatter = *fmtsig; // Find formatter args (if any) for (int i = 0; i < fmtsig->max_args; ++i) if (args.is_set(fmtsig->args[i])) { - spec.format.args.resize(i+1); - spec.format.args[i] = args.get(fmtsig->args[i]); + m_spec.format.args.resize(i+1); + m_spec.format.args[i] = args.get(fmtsig->args[i]); } // NOTE: This check isn't complete yet. - if (spec.format.args.size() < fmtsig->min_args) - Log(0).stream() << "cali-query: Insufficient arguments for formatter " << fmtsig->name << std::endl; + if (m_spec.format.args.size() < fmtsig->min_args) { + m_error = true; + m_error_msg = std::string("Insufficient arguments for formatter ") + fmtsig->name; + return false; + } + break; } } - return spec; + return true; } } diff --git a/src/tools/cali-query/query_common.h b/src/tools/cali-query/query_common.h index d8689b7fb..b93bfcba3 100644 --- a/src/tools/cali-query/query_common.h +++ b/src/tools/cali-query/query_common.h @@ -51,8 +51,27 @@ namespace cali class CaliperMetadataAccessInterface; /// \brief Create QuerySpec from command-line arguments -QuerySpec -spec_from_args(const util::Args& args); +class QueryArgsParser { + bool m_error; + std::string m_error_msg; + QuerySpec m_spec; + +public: + + QueryArgsParser() + : m_error(true), + m_error_msg("query not read") + { } + + /// \brief Get query spec from cali-query command-line arguments. + /// \return Returns \a true if successful, `false` in case of error. + bool parse_args(const util::Args& args); + + bool error() const { return m_error; } + std::string error_msg() const { return m_error_msg; } + + QuerySpec spec() const { return m_spec; } +}; /// \class SnapshotFilterStep /// \brief Basically the chain link in the processing chain.