-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/planop parameter serialization #341
base: master
Are you sure you want to change the base?
Changes from 4 commits
6ed1d56
7414940
d734577
b612f65
1c92480
fe50fff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
#define SRC_LIB_ACCESS_TABLELOAD_H_ | ||
|
||
#include "access/system/PlanOperation.h" | ||
#include "helper/serialization.h" | ||
|
||
namespace hyrise { | ||
namespace access { | ||
|
@@ -14,12 +15,22 @@ class TableLoad : public PlanOperation { | |
friend class LoadTests_simple_load_op_Test; | ||
friend class LoadTests_simple_unloadall_op_Test; | ||
|
||
public: | ||
struct Parameters { | ||
std::string type, table, filename; | ||
std::optional<std::string> header, header_string, delimiter, path; | ||
std::optional<bool> unsafe, raw; | ||
|
||
SERIALIZE(type, table, filename, header, header_string, delimiter, path, unsafe, raw) | ||
}; | ||
|
||
public: | ||
TableLoad(); | ||
TableLoad(const Parameters& parameters); | ||
virtual ~TableLoad(); | ||
|
||
void executePlanOperation(); | ||
static std::shared_ptr<PlanOperation> parse(const Json::Value& data); | ||
|
||
const std::string vname(); | ||
void setTableName(const std::string& tablename); | ||
void setFileName(const std::string& filename); | ||
|
@@ -34,16 +45,15 @@ class TableLoad : public PlanOperation { | |
|
||
private: | ||
std::string _table_name; | ||
std::string _header_file_name; | ||
std::string _file_name; | ||
std::string _path; | ||
std::string _header_string; | ||
std::string _delimiter; | ||
bool _hasDelimiter; | ||
bool _binary; | ||
bool _unsafe; | ||
bool _raw; | ||
std::optional<std::string> _header_file_name; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to list the members of the parameter struct twice? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We wouldn't necessarily need them twice, we could just (as you suggested below) use the Parameters struct as member. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in fe50fff. |
||
std::optional<std::string> _header_string; | ||
std::optional<std::string> _delimiter; | ||
std::optional<std::string> _path; | ||
std::optional<bool> _unsafe; | ||
std::optional<bool> _raw; | ||
bool _nonvolatile; | ||
bool _binary; | ||
}; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,11 @@ | |
#include <string> | ||
#include <stdexcept> | ||
#include <mutex> | ||
#include <sstream> | ||
|
||
#include <json.h> | ||
|
||
#include "helper/cereal/HyriseCerealJsonArchive.h" | ||
#include "access/system/BasicParser.h" | ||
|
||
const std::string autojsonReferenceTableId = "-1"; | ||
|
@@ -38,6 +40,7 @@ struct AbstractQueryParserFactory { | |
|
||
struct parse_construct {}; | ||
struct default_construct {}; | ||
struct cereal_construct {}; | ||
|
||
template <typename T, typename parse_construction> | ||
struct QueryParserFactory; | ||
|
@@ -56,6 +59,22 @@ struct QueryParserFactory<T, default_construct> : public AbstractQueryParserFact | |
} | ||
}; | ||
|
||
template <typename T> | ||
struct QueryParserFactory<T, cereal_construct> : public AbstractQueryParserFactory { | ||
|
||
virtual std::shared_ptr<PlanOperation> parse(const Json::Value& data) { | ||
std::stringstream ss; | ||
ss << data.toStyledString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
cereal::JSONInputArchive archive(ss); | ||
|
||
typename T::Parameters params; | ||
params.serialize(archive); | ||
|
||
return std::make_shared<T>(params); | ||
} | ||
}; | ||
|
||
/* | ||
* The Query Parser parses a given Json Value to create a plan operation | ||
* | ||
|
@@ -108,6 +127,12 @@ class QueryParser { | |
return true; | ||
} | ||
|
||
template <typename T> | ||
static bool registerSerializablePlanOperation(const std::string& name) { | ||
QueryParser::instance()._factory[name] = new QueryParserFactory<T, cereal_construct>(); | ||
return true; | ||
} | ||
|
||
std::shared_ptr<PlanOperation> parse(std::string name, const Json::Value& d); | ||
|
||
static QueryParser& instance(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just leave Parameters as a member? The way this is currently implemented enforces a lot of extra typing for no extra value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See fe50fff.