Skip to content

Commit

Permalink
Filter by null and not null
Browse files Browse the repository at this point in the history
* Add cases/constants to handle null/not null to emscripten.cpp,
  base.cpp/.h, scalar.cpp, and python.h
* Fix table.cpp so that null values are not skipped when filtering by null
* Alter _is_valid_filter in emscripten.cpp with a special case for
  null/not null so it doesnt reject those filters incorrectly
* Refactor constants.js to allow for accessing filter operators in a
  non magic string way
* Add special condition to row.js filter update so that the operand
  field is hidden when selecting null/not null filter
* Add special condition to perspective.js is_valid_filter so null/not
  null is always valid (operand is irrelevant in this case)
* Add Specs
  • Loading branch information
jspillers committed Aug 1, 2019
1 parent 87e63b4 commit 7c38a9a
Show file tree
Hide file tree
Showing 15 changed files with 178 additions and 20 deletions.
10 changes: 10 additions & 0 deletions cpp/perspective/src/cpp/base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,12 @@ filter_op_to_str(t_filter_op op) {
case FILTER_OP_AND: {
return "and";
} break;
case FILTER_OP_IS_NULL: {
return "is null";
} break;
case FILTER_OP_IS_NOT_NULL: {
return "is not null";
} break;
case FILTER_OP_IS_VALID: {
return "is not None";
} break;
Expand Down Expand Up @@ -334,6 +340,10 @@ str_to_filter_op(std::string str) {
return t_filter_op::FILTER_OP_AND;
} else if (str == "|") {
return t_filter_op::FILTER_OP_OR;
} else if (str == "is null") {
return t_filter_op::FILTER_OP_IS_NULL;
} else if (str == "is not null") {
return t_filter_op::FILTER_OP_IS_NOT_NULL;
} else if (str == "is not None") {
return t_filter_op::FILTER_OP_IS_VALID;
} else if (str == "is None") {
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/data_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ t_data_table::filter_cpp(t_filter_op combiner, const std::vector<t_fterm>& fterm
tval = ft(cell_val);
}

if (!cell_val.is_valid() || !tval) {
if ((ft.m_op != FILTER_OP_IS_NULL && !cell_val.is_valid()) || !tval) {
pass = false;
break;
}
Expand Down
16 changes: 13 additions & 3 deletions cpp/perspective/src/cpp/emscripten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1300,8 +1300,14 @@ namespace binding {

template <>
bool
is_valid_filter(t_dtype type, t_val date_parser, t_val filter_term) {
if (type == DTYPE_DATE || type == DTYPE_TIME) {
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) {
return true;
} else if (type == DTYPE_DATE || type == DTYPE_TIME) {
t_val parsed_date = date_parser.call<t_val>("parse", filter_term);
return has_value(parsed_date);
} else {
Expand All @@ -1326,6 +1332,10 @@ namespace binding {
terms.push_back(mktscalar(get_interned_cstr(term.c_str())));
}
} break;
case FILTER_OP_IS_NULL:
case FILTER_OP_IS_NOT_NULL: {
terms.push_back(mktscalar(0));
} break;
default: {
switch (type) {
case DTYPE_INT32: {
Expand Down Expand Up @@ -1394,7 +1404,7 @@ namespace binding {
t_dtype type = schema.get_dtype(f[0].as<std::string>());

// validate the filter before it goes into the core engine
if (is_valid_filter(type, date_parser, f[2])) {
if (is_valid_filter(type, date_parser, f[2], f[1])) {
filter.push_back(make_filter_term(type, date_parser, f));
}
}
Expand Down
6 changes: 6 additions & 0 deletions cpp/perspective/src/cpp/scalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,12 @@ t_tscalar::cmp(t_filter_op op, const t_tscalar& other) const {
case FILTER_OP_CONTAINS: {
return value.contains(other);
} break;
case FILTER_OP_IS_NULL: {
return m_status != STATUS_VALID;
} break;
case FILTER_OP_IS_NOT_NULL: {
return m_status == STATUS_VALID;
} break;
case FILTER_OP_IS_VALID: {
return m_status == STATUS_VALID;
} break;
Expand Down
2 changes: 1 addition & 1 deletion cpp/perspective/src/cpp/table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,4 @@ Table::process_index_column(t_data_table& data_table) {
}
}

} // namespace perspective
} // namespace perspective
2 changes: 2 additions & 0 deletions cpp/perspective/src/include/perspective/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ enum t_filter_op {
FILTER_OP_IN,
FILTER_OP_NOT_IN,
FILTER_OP_AND,
FILTER_OP_IS_NULL,
FILTER_OP_IS_NOT_NULL,
FILTER_OP_IS_VALID,
FILTER_OP_IS_NOT_VALID
};
Expand Down
13 changes: 8 additions & 5 deletions cpp/perspective/src/include/perspective/binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,19 +231,22 @@ namespace binding {
*/

/**
* @brief For date/datetime values, is the filter term a valid date?
* @brief is the filter term and operand combination valid?
*
* Otherwise, make sure the filter term is not null/undefined.
* If the operand is null/not null then it is always valid.
* Is the filter term is a valid date? Otherwise, make sure
* the filter term is not null/undefined.
*
* @tparam T
* @param type
* @param date_parser
* @param filter
* @param filter_term
* @param filter_operand
* @return true
* @return false
*/
template <typename T>
bool is_valid_filter(t_dtype type, T date_parser, T filter_term);
bool is_valid_filter(t_dtype type, T date_parser, T filter_term, T filter_operand);

/**
* @brief Create a filter by parsing the filter term from the binding language.
Expand Down Expand Up @@ -352,4 +355,4 @@ namespace binding {
} // end namespace binding
} // end namespace perspective

#endif
#endif
6 changes: 3 additions & 3 deletions examples/simple/csv.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<!--
Copyright (c) 2017, the Perspective Authors.
This file is part of the Perspective library, distributed under the terms of
the Apache License 2.0. The full license can be found in the LICENSE file.
Expand Down Expand Up @@ -40,4 +40,4 @@

</body>

</html>
</html>
10 changes: 8 additions & 2 deletions packages/perspective-viewer/src/js/row.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class Row extends HTMLElement {
if (!this.hasAttribute("aggregate")) {
this.setAttribute("aggregate", get_type_config(type).aggregate);
}

let filter_operand = this.shadowRoot.querySelector("#filter_operand");
this._callback = event => this._update_filter(event);
filter_operand.addEventListener("keyup", this._callback.bind(this));
Expand Down Expand Up @@ -128,7 +129,12 @@ class Row extends HTMLElement {
if (!this._initialized) {
filter_input.value = operand;
}
filter_input.style.width = get_text_width(operand, 30);
if (filter_dropdown.value === perspective.FILTER_OPERATORS.isNull || filter_dropdown.value === perspective.FILTER_OPERATORS.isNotNull) {
filter_input.style.display = "none";
} else {
filter_input.style.display = "inline-block";
filter_input.style.width = get_text_width(operand, 30);
}
}

set aggregate(a) {
Expand Down Expand Up @@ -177,7 +183,7 @@ class Row extends HTMLElement {
case "string":
default:
}
if (filter_operator.value === "in" || filter_operator.value === "not in") {
if (filter_operator.value === perspective.FILTER_OPERATORS.isIn || filter_operator.value === perspective.FILTER_OPERATORS.isNotIn) {
val = val.split(",").map(x => x.trim());
}
this.setAttribute("filter", JSON.stringify({operator: filter_operator.value, operand: val}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export class PerspectiveElement extends StateElement {
const exclamation = node.shadowRoot.getElementById("row_exclamation");
const {operator, operand} = JSON.parse(node.getAttribute("filter"));
const filter = [node.getAttribute("name"), operator, operand];
if ((await this._table.is_valid_filter(filter)) && operand !== "") {
if (await this._table.is_valid_filter(filter)) {
filters.push(filter);
node.title = "";
operandNode.style.borderColor = "";
Expand Down
66 changes: 62 additions & 4 deletions packages/perspective/src/js/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,71 @@ export const TYPE_AGGREGATES = {
date: STRING_AGGREGATES
};

const BOOLEAN_FILTERS = ["&", "|", "==", "!=", "or", "and"];
export const FILTER_OPERATORS = {
lessThan: "<",
greaterThan: ">",
equals: "==",
doesNotEqual: "==",
lessThanOrEquals: "<=",
greaterThanOrEquals: ">=",
doesNotEqual: "!=",
isNull: "is null",
isNotNull: "is not null",
isIn: "in",
isNotIn: "not in",
contains: "contains",
bitwiseAnd: "&",
bitwiseOr: "|",
and: "and",
or: "or",
beginsWith: "begins with",
endsWith: "ends with"
};

const BOOLEAN_FILTERS = [
FILTER_OPERATORS.bitwiseAnd,
FILTER_OPERATORS.bitwiseOr,
FILTER_OPERATORS.equals,
FILTER_OPERATORS.doesNotEqual,
FILTER_OPERATORS.or,
FILTER_OPERATORS.and,
FILTER_OPERATORS.isNull,
FILTER_OPERATORS.isNotNull
];

const NUMBER_FILTERS = ["<", ">", "==", "<=", ">=", "!=", "is nan", "is not nan"];
const NUMBER_FILTERS = [
FILTER_OPERATORS.lessThan,
FILTER_OPERATORS.greaterThan,
FILTER_OPERATORS.equals,
FILTER_OPERATORS.lessThanOrEquals,
FILTER_OPERATORS.greaterThanOrEquals,
FILTER_OPERATORS.doesNotEqual,
FILTER_OPERATORS.isNull,
FILTER_OPERATORS.isNotNull
];

const STRING_FILTERS = ["==", "contains", "!=", "in", "not in", "begins with", "ends with"];
const STRING_FILTERS = [
FILTER_OPERATORS.equals,
FILTER_OPERATORS.contains,
FILTER_OPERATORS.doesNotEqual,
FILTER_OPERATORS.isIn,
FILTER_OPERATORS.isNotIn,
FILTER_OPERATORS.beginsWith,
FILTER_OPERATORS.endsWith,
FILTER_OPERATORS.isNull,
FILTER_OPERATORS.isNotNull
];

const DATETIME_FILTERS = ["<", ">", "==", "<=", ">=", "!="];
const DATETIME_FILTERS = [
FILTER_OPERATORS.lessThan,
FILTER_OPERATORS.greaterThan,
FILTER_OPERATORS.equals,
FILTER_OPERATORS.lessThanOrEquals,
FILTER_OPERATORS.greaterThanOrEquals,
FILTER_OPERATORS.doesNotEqual,
FILTER_OPERATORS.isNull,
FILTER_OPERATORS.isNotNull
];

export const COLUMN_SEPARATOR_STRING = "|";

Expand Down
5 changes: 5 additions & 0 deletions packages/perspective/src/js/perspective.js
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,11 @@ export default function(Module) {
};

table.prototype._is_valid_filter = function(filter) {
// isNull and isNotNull filter operators are always valid and apply to all schema types
if (filter[1] === perspective.FILTER_OPERATORS.isNull || filter[1] === perspective.FILTER_OPERATORS.isNotNull) {
return true;
}

if (filter[2] === null) {
return false;
}
Expand Down
38 changes: 38 additions & 0 deletions packages/perspective/test/js/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,44 @@ module.exports = perspective => {
});
});

describe("is null", function() {
it("returns the correct null cells for string column", async function() {
const table = perspective.table([{x: 1, y: null}, {x: 2, y: null}, {x: 3, y: "x"}, {x: 4, y: "x"}, {x: 1, y: "y"}, {x: 2, y: "x"}, {x: 3, y: "y"}]);
const view = table.view({
filter: [["y", "is null"]]
});
const answer = [{x: 1, y: null}, {x: 2, y: null}];
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("returns the correct null cells for integer column", async function() {
const table = perspective.table([{x: 1, y: null}, {x: 2, y: null}, {x: 3, y: 1}, {x: 4, y: 2}, {x: 1, y: 3}, {x: 2, y: 4}, {x: 3, y: 5}]);
const view = table.view({
filter: [["y", "is null"]]
});
const answer = [{x: 1, y: null}, {x: 2, y: null}];
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("returns the correct null cells for datetime column", async function() {
const table = perspective.table([{x: 1, y: null}, {x: 2, y: null}, {x: 3, y: "1/1/2019"}, {x: 4, y: "1/1/2019"}, {x: 1, y: "1/1/2019"}, {x: 2, y: "1/1/2019"}, {x: 3, y: "1/1/2019"}]);
const view = table.view({
filter: [["y", "is null"]]
});
const answer = [{x: 1, y: null}, {x: 2, y: null}];
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});
});

describe("nulls", function() {
it("x > 2", async function() {
var table = perspective.table([{x: 3, y: 1}, {x: 2, y: 1}, {x: null, y: 1}, {x: null, y: 1}, {x: 4, y: 2}, {x: null, y: 2}]);
Expand Down
18 changes: 18 additions & 0 deletions packages/perspective/test/js/pivot_nulls.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,24 @@ module.exports = perspective => {
table.delete();
});

it("aggregates that return NaN render correctly", async function() {
const dataWithNull1 = [{name: "Homer", value: 3}, {name: "Homer", value: 1}, {name: "Marge", value: null}, {name: "Marge", value: null}];

var table = perspective.table(dataWithNull1);

var view = table.view({
row_pivots: ["name"],
aggregates: {value: "avg"}
});

const answer = [{__ROW_PATH__: [], name: 4, value: 2}, {__ROW_PATH__: ["Homer"], name: 2, value: 2}, {__ROW_PATH__: ["Marge"], name: 2, value: null}];

let results = await view.to_json();
expect(results).toEqual(answer);
view.delete();
table.delete();
});

it("aggregates nulls correctly", async function() {
const data = [{x: "AAAAAAAAAAAAAA"}, {x: "AAAAAAAAAAAAAA"}, {x: "AAAAAAAAAAAAAA"}, {x: null}, {x: null}, {x: "BBBBBBBBBBBBBB"}, {x: "BBBBBBBBBBBBBB"}, {x: "BBBBBBBBBBBBBB"}];
const tbl = perspective.table(data);
Expand Down
2 changes: 2 additions & 0 deletions python/table/perspective/include/perspective/python.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ BOOST_PYTHON_MODULE(libbinding)
.value("FILTER_OP_IN", perspective::FILTER_OP_IN)
.value("FILTER_OP_NOT_IN", perspective::FILTER_OP_NOT_IN)
.value("FILTER_OP_AND", perspective::FILTER_OP_AND)
.value("FILTER_OP_IS_NULL", perspective::FILTER_OP_IS_NULL)
.value("FILTER_OP_IS_NOT_NULL", perspective::FILTER_OP_IS_NOT_NULL)
.value("FILTER_OP_IS_VALID", perspective::FILTER_OP_IS_VALID)
.value("FILTER_OP_IS_NOT_VALID", perspective::FILTER_OP_IS_NOT_VALID);

Expand Down

0 comments on commit 7c38a9a

Please sign in to comment.