Skip to content
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.

Sign up for GitHub

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

Refactor C++ #707

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Refactor C++ #707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cpp/perspective/src/cpp/base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ filter_op_to_str(t_filter_op op) {
}

t_filter_op
str_to_filter_op(std::string str) {
str_to_filter_op(const std::string& str) {
if (str == "<") {
return t_filter_op::FILTER_OP_LT;
} else if (str == "<=") {
Expand All @@ -347,7 +347,7 @@ str_to_filter_op(std::string str) {
return t_filter_op::FILTER_OP_NOT_IN;
} else if (str == "&" || str == "and") {
return t_filter_op::FILTER_OP_AND;
} else if (str == "|") {
} else if (str == "|" || str == "or") {
return t_filter_op::FILTER_OP_OR;
} else if (str == "is null") {
return t_filter_op::FILTER_OP_IS_NULL;
Expand All @@ -365,7 +365,7 @@ str_to_filter_op(std::string str) {
}

t_sorttype
str_to_sorttype(std::string str) {
str_to_sorttype(const std::string& str) {
if (str == "none") {
return SORTTYPE_NONE;
} else if (str == "asc" || str == "col asc") {
Expand All @@ -383,7 +383,7 @@ str_to_sorttype(std::string str) {
}

t_aggtype
str_to_aggtype(std::string str) {
str_to_aggtype(const std::string& str) {
if (str == "distinct count" || str == "distinctcount" || str == "distinct"
|| str == "distinct_count") {
return t_aggtype::AGGTYPE_DISTINCT_COUNT;
Expand Down
75 changes: 38 additions & 37 deletions cpp/perspective/src/cpp/emscripten.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ namespace binding {
}

void
_fill_col_date(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
_fill_col_date(t_data_accessor accessor, std::shared_ptr<t_column> col, const std::string& name,
std::int32_t cidx, t_dtype type, bool is_arrow, bool is_update) {
t_uindex nrows = col->size();

Expand Down Expand Up @@ -748,7 +748,7 @@ namespace binding {
}

void
_fill_col_bool(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
_fill_col_bool(t_data_accessor accessor, std::shared_ptr<t_column> col, const std::string& name,
std::int32_t cidx, t_dtype type, bool is_arrow, bool is_update) {
t_uindex nrows = col->size();

Expand Down Expand Up @@ -798,7 +798,7 @@ namespace binding {
}

void
_fill_col_string(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
_fill_col_string(t_data_accessor accessor, std::shared_ptr<t_column> col, const std::string& name,
std::int32_t cidx, t_dtype type, bool is_arrow, bool is_update) {

t_uindex nrows = col->size();
Expand Down Expand Up @@ -869,7 +869,7 @@ namespace binding {
}

void
_fill_col_int64(t_data_accessor accessor, t_data_table& tbl, std::shared_ptr<t_column> col, std::string name,
_fill_col_int64(t_data_accessor accessor, t_data_table& tbl, std::shared_ptr<t_column> col, const std::string& name,
std::int32_t cidx, t_dtype type, bool is_arrow, bool is_update) {
t_uindex nrows = col->size();

Expand Down Expand Up @@ -911,7 +911,7 @@ namespace binding {

void
_fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type,
std::shared_ptr<t_column> col, const std::string& name, std::int32_t cidx, t_dtype type,
bool is_arrow, bool is_update) {
t_uindex nrows = col->size();

Expand Down Expand Up @@ -1174,7 +1174,7 @@ namespace binding {
*/
void
_fill_data_helper(t_data_accessor accessor, t_data_table& tbl,
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type,
std::shared_ptr<t_column> col, const std::string& name, std::int32_t cidx, t_dtype type,
bool is_arrow, bool is_update) {
switch (type) {
case DTYPE_INT64: {
Expand Down Expand Up @@ -1269,7 +1269,7 @@ namespace binding {
template <>
std::shared_ptr<Table>
make_table(t_val table, t_data_accessor accessor, t_val computed,
std::uint32_t limit, std::string index, t_op op, bool is_update, bool is_arrow) {
std::uint32_t limit, const std::string& index, t_op op, bool is_update, bool is_arrow) {
std::vector<std::string> column_names;
std::vector<t_dtype> data_types;

Expand Down Expand Up @@ -1377,14 +1377,11 @@ namespace binding {

template <>
bool
is_valid_filter(t_dtype type, t_val date_parser, t_val filter_term, t_val filter_operand) {
std::string comp_str = filter_operand.as<std::string>();
t_filter_op comp = str_to_filter_op(comp_str);

if (comp == t_filter_op::FILTER_OP_IS_NULL
|| comp == t_filter_op::FILTER_OP_IS_NOT_NULL) {
is_valid_filter(t_dtype column_type, t_val date_parser, t_filter_op filter_operator, t_val filter_term) {
if (filter_operator == t_filter_op::FILTER_OP_IS_NULL
|| filter_operator == t_filter_op::FILTER_OP_IS_NOT_NULL) {
return true;
} else if (type == DTYPE_DATE || type == DTYPE_TIME) {
} else if (column_type == DTYPE_DATE || column_type == DTYPE_TIME) {
t_val parsed_date = date_parser.call<t_val>("parse", filter_term);
return has_value(parsed_date);
} else {
Expand All @@ -1394,17 +1391,15 @@ namespace binding {

template <>
std::tuple<std::string, std::string, std::vector<t_tscalar>>
make_filter_term(t_dtype type, t_val date_parser, std::vector<t_val> filter) {
std::string col = filter[0].as<std::string>();
std::string comp_str = filter[1].as<std::string>();
t_filter_op comp = str_to_filter_op(comp_str);
make_filter_term(t_dtype column_type, t_val date_parser, const std::string column_name, const std::string& filter_op_str, t_val filter_term) {
t_filter_op filter_op = str_to_filter_op(filter_op_str);
std::vector<t_tscalar> terms;

switch (comp) {
switch (filter_op) {
case FILTER_OP_NOT_IN:
case FILTER_OP_IN: {
std::vector<std::string> filter_terms
= vecFromArray<t_val, std::string>(filter[2]);
= vecFromArray<t_val, std::string>(filter_term);
for (auto term : filter_terms) {
terms.push_back(mktscalar(get_interned_cstr(term.c_str())));
}
Expand All @@ -1414,35 +1409,35 @@ namespace binding {
terms.push_back(mktscalar(0));
} break;
default: {
switch (type) {
switch (column_type) {
case DTYPE_INT32: {
terms.push_back(mktscalar(filter[2].as<std::int32_t>()));
terms.push_back(mktscalar(filter_term.as<std::int32_t>()));
} break;
case DTYPE_INT64:
case DTYPE_FLOAT64: {
terms.push_back(mktscalar(filter[2].as<double>()));
terms.push_back(mktscalar(filter_term.as<double>()));
} break;
case DTYPE_BOOL: {
terms.push_back(mktscalar(filter[2].as<bool>()));
terms.push_back(mktscalar(filter_term.as<bool>()));
} break;
case DTYPE_DATE: {
t_val parsed_date = date_parser.call<t_val>("parse", filter[2]);
t_val parsed_date = date_parser.call<t_val>("parse", filter_term);
terms.push_back(mktscalar(jsdate_to_t_date(parsed_date)));
} break;
case DTYPE_TIME: {
t_val parsed_date = date_parser.call<t_val>("parse", filter[2]);
t_val parsed_date = date_parser.call<t_val>("parse", filter_term);
terms.push_back(mktscalar(t_time(static_cast<std::int64_t>(
parsed_date.call<t_val>("getTime").as<double>()))));
} break;
default: {
terms.push_back(
mktscalar(get_interned_cstr(filter[2].as<std::string>().c_str())));
mktscalar(get_interned_cstr(filter_term.as<std::string>().c_str())));
}
}
}
}

return std::make_tuple(col, comp_str, terms);
return std::make_tuple(column_name, filter_op_str, terms);
}

template <>
Expand Down Expand Up @@ -1478,15 +1473,21 @@ namespace binding {
std::vector<std::tuple<std::string, std::string, std::vector<t_tscalar>>> filter;

for (auto f : js_filter) {
t_dtype type = schema.get_dtype(f.at(0).as<std::string>());

// parse filter details
std::string column_name = f.at(0).as<std::string>();
std::string filter_op_str = f.at(1).as<std::string>();
t_dtype column_type = schema.get_dtype(column_name);
t_filter_op filter_operator = str_to_filter_op(filter_op_str);

// validate the filter before it goes into the core engine
t_val filter_term = t_val::null();
if (f.size() > 2) {
// null/not null filters do not have a filter term
filter_term = f.at(2);
}
if (is_valid_filter(type, date_parser, filter_term, f.at(1))) {
filter.push_back(make_filter_term(type, date_parser, f));

if (is_valid_filter(column_type, date_parser, filter_operator, filter_term)) {
filter.push_back(make_filter_term(column_type, date_parser, column_name, filter_op_str, filter_term));
}
}

Expand All @@ -1511,7 +1512,7 @@ namespace binding {

template <typename CTX_T>
std::shared_ptr<View<CTX_T>>
make_view(std::shared_ptr<Table> table, std::string name, std::string separator,
make_view(std::shared_ptr<Table> table, const std::string& name, const std::string& separator,
t_val view_config, t_val date_parser) {
auto schema = table->get_schema();
t_view_config config = make_view_config<t_val>(schema, date_parser, view_config);
Expand All @@ -1531,7 +1532,7 @@ namespace binding {
template <>
std::shared_ptr<t_ctx0>
make_context(std::shared_ptr<Table> table, const t_schema& schema,
const t_view_config& view_config, std::string name) {
const t_view_config& view_config, const std::string& name) {
auto columns = view_config.get_columns();
auto filter_op = view_config.get_filter_op();
auto fterm = view_config.get_fterm();
Expand All @@ -1553,7 +1554,7 @@ namespace binding {
template <>
std::shared_ptr<t_ctx1>
make_context(std::shared_ptr<Table> table, const t_schema& schema,
const t_view_config& view_config, std::string name) {
const t_view_config& view_config, const std::string& name) {
auto row_pivots = view_config.get_row_pivots();
auto aggspecs = view_config.get_aggspecs();
auto filter_op = view_config.get_filter_op();
Expand Down Expand Up @@ -1584,7 +1585,7 @@ namespace binding {
template <>
std::shared_ptr<t_ctx2>
make_context(std::shared_ptr<Table> table, const t_schema& schema,
const t_view_config& view_config, std::string name) {
const t_view_config& view_config, const std::string& name) {
bool column_only = view_config.is_column_only();
auto row_pivots = view_config.get_row_pivots();
auto column_pivots = view_config.get_column_pivots();
Expand Down Expand Up @@ -1639,7 +1640,7 @@ namespace binding {

template <>
t_val
get_column_data(std::shared_ptr<t_data_table> table, std::string colname) {
get_column_data(std::shared_ptr<t_data_table> table, const std::string& colname) {
t_val arr = t_val::array();
auto col = table->get_column(colname);
for (auto idx = 0; idx < col->size(); ++idx) {
Expand Down
46 changes: 23 additions & 23 deletions cpp/perspective/src/cpp/table.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,15 @@ Table::Table(std::shared_ptr<t_pool> pool, std::vector<std::string> column_names
: m_init(false)
, m_id(GLOBAL_TABLE_ID++)
, m_pool(pool)
, m_column_names(column_names)
, m_data_types(data_types)
, m_column_names(std::move(column_names))
, m_data_types(std::move(data_types))
, m_offset(0)
, m_limit(limit)
, m_index(index)
, m_index(std::move(index))
, m_gnode_set(false) {
validate_columns(column_names);
validate_columns(m_column_names);
}

void
Table::validate_columns(const std::vector<std::string>& column_names) {
bool implicit_index =
std::find(column_names.begin(), column_names.end(), "__INDEX__") != column_names.end();
if (m_index != "") {
// Check if index is valid after getting column names
bool explicit_index
= std::find(column_names.begin(), column_names.end(), m_index) != column_names.end();
if (!explicit_index) {
std::cout << "Specified index " << m_index << " does not exist in data." << std::endl;
PSP_COMPLAIN_AND_ABORT("Specified index '" + m_index + "' does not exist in data.");
}
if (explicit_index && implicit_index) {
std::cout << "Specified index " << m_index << " twice - ignoring implicit __INDEX__" << std::endl;
PSP_COMPLAIN_AND_ABORT("Specified index '" + m_index + "' twice; ignoring implicit index.");
}
}
}

void
Table::init(t_data_table& data_table, std::uint32_t row_count, const t_op op) {
/**
Expand Down Expand Up @@ -181,6 +162,25 @@ Table::set_data_types(const std::vector<t_dtype>& data_types) {
m_data_types = data_types;
}

void
Table::validate_columns(const std::vector<std::string>& column_names) {
bool implicit_index =
std::find(column_names.begin(), column_names.end(), "__INDEX__") != column_names.end();
if (m_index != "") {
// Check if index is valid after getting column names
bool explicit_index
= std::find(column_names.begin(), column_names.end(), m_index) != column_names.end();
if (!explicit_index) {
std::cout << "Specified index " << m_index << " does not exist in data." << std::endl;
PSP_COMPLAIN_AND_ABORT("Specified index '" + m_index + "' does not exist in data.");
}
if (explicit_index && implicit_index) {
std::cout << "Specified index " << m_index << " twice - ignoring implicit __INDEX__" << std::endl;
PSP_COMPLAIN_AND_ABORT("Specified index '" + m_index + "' twice; ignoring implicit index.");
}
}
}

void
Table::process_op_column(t_data_table& data_table, const t_op op) {
auto op_col = data_table.add_column("psp_op", DTYPE_UINT8, false);
Expand Down
6 changes: 3 additions & 3 deletions cpp/perspective/src/cpp/view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ View<CTX_T>::View(std::shared_ptr<Table> table, std::shared_ptr<CTX_T> ctx, std:
std::string separator, t_view_config view_config)
: m_table(table)
, m_ctx(ctx)
, m_name(name)
, m_separator(separator)
, m_name(std::move(name))
, m_separator(std::move(separator))
, m_col_offset(0)
, m_view_config(view_config) {
, m_view_config(std::move(view_config)) {

m_row_pivots = m_view_config.get_row_pivots();
m_column_pivots = m_view_config.get_column_pivots();
Expand Down
14 changes: 7 additions & 7 deletions cpp/perspective/src/cpp/view_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ t_view_config::t_view_config(std::vector<std::string> row_pivots,
std::vector<std::tuple<std::string, std::string, std::vector<t_tscalar>>> filter,
std::vector<std::vector<std::string>> sort, std::string filter_op, bool column_only)
: m_init(false)
, m_row_pivots(row_pivots)
, m_column_pivots(column_pivots)
, m_aggregates(aggregates)
, m_columns(columns)
, m_filter(filter)
, m_sort(sort)
, m_row_pivots(std::move(row_pivots))
, m_column_pivots(std::move(column_pivots))
, m_aggregates(std::move(aggregates))
, m_columns(std::move(columns))
, m_filter(std::move(filter))
, m_sort(std::move(sort))
, m_row_pivot_depth(-1)
, m_column_pivot_depth(-1)
, m_filter_op(filter_op)
, m_filter_op(std::move(filter_op))
, m_column_only(column_only) {}

void
Expand Down
6 changes: 3 additions & 3 deletions cpp/perspective/src/include/perspective/base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ enum t_filter_op {
};

PERSPECTIVE_EXPORT std::string filter_op_to_str(t_filter_op op);
PERSPECTIVE_EXPORT t_filter_op str_to_filter_op(std::string str);
PERSPECTIVE_EXPORT t_filter_op str_to_filter_op(const std::string& str);

enum t_header { HEADER_ROW, HEADER_COLUMN };

Expand All @@ -202,7 +202,7 @@ enum t_sorttype {
SORTTYPE_DESCENDING_ABS
};

PERSPECTIVE_EXPORT t_sorttype str_to_sorttype(std::string str);
PERSPECTIVE_EXPORT t_sorttype str_to_sorttype(const std::string& str);
PERSPECTIVE_EXPORT std::string sorttype_to_str(t_sorttype type);

enum t_aggtype {
Expand Down Expand Up @@ -239,7 +239,7 @@ enum t_aggtype {
AGGTYPE_PCT_SUM_GRAND_TOTAL
};

PERSPECTIVE_EXPORT t_aggtype str_to_aggtype(std::string str);
PERSPECTIVE_EXPORT t_aggtype str_to_aggtype(const std::string& str);
PERSPECTIVE_EXPORT t_aggtype _get_default_aggregate(t_dtype dtype);
PERSPECTIVE_EXPORT std::string _get_default_aggregate_string(t_dtype dtype);

Expand Down
Loading