Skip to content

Commit

Permalink
Merge pull request #707 from finos/cpp-refactor
Browse files Browse the repository at this point in the history 
Refactor C++
  • Loading branch information
@texodus
texodus authored Sep 5, 2019
2 parents 8e1fc3f + cd0080b commit aa32e7c
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 117 deletions.
8 changes: 4 additions & 4 deletions cpp/perspective/src/cpp/base.cpp
View file
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
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
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
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
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
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

0 comments on commit aa32e7c

Please sign in to comment.