diff --git a/std-bits/table/src/main/java/org/enso/table/data/column/storage/BoolStorage.java b/std-bits/table/src/main/java/org/enso/table/data/column/storage/BoolStorage.java index 6af37d21545..5653e3cc347 100644 --- a/std-bits/table/src/main/java/org/enso/table/data/column/storage/BoolStorage.java +++ b/std-bits/table/src/main/java/org/enso/table/data/column/storage/BoolStorage.java @@ -335,13 +335,20 @@ public final class BoolStorage extends Storage { Object arg, MapOperationProblemAggregator problemAggregator) { if (arg == null) { - return BoolStorage.makeEmpty(storage.size); - } else if (arg instanceof Boolean v) { - if (v) { - return storage; + if (storage.negated) { + var newMissing = new BitSet(storage.size); + newMissing.flip(0, storage.size); + newMissing.xor(storage.values); + return new BoolStorage(storage.values, newMissing, storage.size, true); } else { - return new BoolStorage(new BitSet(), storage.isMissing, storage.size, false); + var newMissing = storage.isMissing.get(0, storage.size); + newMissing.or(storage.values); + return new BoolStorage(new BitSet(), newMissing, storage.size, false); } + } else if (arg instanceof Boolean v) { + return v + ? storage + : new BoolStorage(new BitSet(), new BitSet(), storage.size, false); } else { throw new UnexpectedTypeException("a Boolean"); } @@ -352,29 +359,41 @@ public final class BoolStorage extends Storage { BoolStorage storage, Storage arg, MapOperationProblemAggregator problemAggregator) { - if (arg instanceof BoolStorage v) { - BitSet missing = v.isMissing.get(0, storage.size); - missing.or(storage.isMissing); - BitSet out = v.values.get(0, storage.size); - boolean negated; - if (storage.negated && v.negated) { - out.or(storage.values); - negated = true; - } else if (storage.negated) { - out.andNot(storage.values); - negated = false; - } else if (v.negated) { - out.flip(0, storage.size); - out.and(storage.values); - negated = false; - } else { - out.and(storage.values); - negated = false; - } - return new BoolStorage(out, missing, storage.size, negated); - } else { + if (!(arg instanceof BoolStorage v)) { throw new UnexpectedColumnTypeException("Boolean"); } + + BitSet out = v.values.get(0, storage.size); + boolean negated; + if (storage.negated && v.negated) { + out.or(storage.values); + negated = true; + } else if (storage.negated) { + out.andNot(storage.values); + negated = false; + } else if (v.negated) { + out.flip(0, storage.size); + out.and(storage.values); + negated = false; + } else { + out.and(storage.values); + negated = false; + } + + BitSet missing = BitSets.makeDuplicate(storage.isMissing); + missing.or(v.isMissing); + int current = missing.nextSetBit(0); + while (current != -1) { + var value = negated != out.get(current); + if (!value + && (storage.getItemBoxed(current) == Boolean.FALSE + || v.getItemBoxed(current) == Boolean.FALSE)) { + missing.clear(current); + } + current = missing.nextSetBit(current + 1); + } + + return new BoolStorage(out, missing, storage.size, negated); } }) .add( @@ -385,13 +404,20 @@ public final class BoolStorage extends Storage { Object arg, MapOperationProblemAggregator problemAggregator) { if (arg == null) { - return BoolStorage.makeEmpty(storage.size); - } else if (arg instanceof Boolean v) { - if (v) { - return new BoolStorage(new BitSet(), storage.isMissing, storage.size, true); + if (storage.negated) { + var newMissing = storage.isMissing.get(0, storage.size); + newMissing.or(storage.values); + return new BoolStorage(new BitSet(), newMissing, storage.size, true); } else { - return storage; + var newMissing = new BitSet(storage.size); + newMissing.flip(0, storage.size); + newMissing.xor(storage.values); + return new BoolStorage(storage.values, newMissing, storage.size, false); } + } else if (arg instanceof Boolean v) { + return v + ? new BoolStorage(new BitSet(), new BitSet(), storage.size, true) + : storage; } else { throw new UnexpectedTypeException("a Boolean"); } @@ -402,30 +428,42 @@ public final class BoolStorage extends Storage { BoolStorage storage, Storage arg, MapOperationProblemAggregator problemAggregator) { - if (arg instanceof BoolStorage v) { - BitSet missing = v.isMissing.get(0, storage.size); - missing.or(storage.isMissing); - BitSet out = v.values.get(0, storage.size); - boolean negated; - if (storage.negated && v.negated) { - out.and(storage.values); - negated = true; - } else if (storage.negated) { - out.flip(0, storage.size); - out.and(storage.values); - negated = true; - } else if (v.negated) { - out.flip(0, storage.size); - out.or(storage.values); - negated = false; - } else { - out.or(storage.values); - negated = false; - } - return new BoolStorage(out, missing, storage.size, negated); - } else { + if (!(arg instanceof BoolStorage v)) { throw new UnexpectedColumnTypeException("Boolean"); } + + BitSet out = v.values.get(0, storage.size); + boolean negated; + if (storage.negated && v.negated) { + out.and(storage.values); + negated = true; + } else if (storage.negated) { + out.flip(0, storage.size); + out.and(storage.values); + negated = true; + } else if (v.negated) { + out.flip(0, storage.size); + out.or(storage.values); + negated = false; + } else { + out.or(storage.values); + negated = false; + } + + BitSet missing = BitSets.makeDuplicate(storage.isMissing); + missing.or(v.isMissing); + int current = missing.nextSetBit(0); + while (current != -1) { + var value = negated != out.get(current); + if (value + && (storage.getItemBoxed(current) == Boolean.TRUE + || v.getItemBoxed(current) == Boolean.TRUE)) { + missing.clear(current); + } + current = missing.nextSetBit(current + 1); + } + + return new BoolStorage(out, missing, storage.size, negated); } }) .add( diff --git a/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso index 157930c0b01..2001161169f 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso @@ -162,12 +162,43 @@ spec setup = (x && y).to_vector . should_equal [True, False, False] (x || y.not).to_vector . should_equal [True, True, True] - Test.specify "should return null if one of arguments is missing" pending="TODO null handling" <| - t = table_builder [["X", [True, False, True]]] - x = t.at "X" - nulls = [Nothing, Nothing, Nothing, Nothing] - (x && Nothing).to_vector . should_equal nulls - (x || Nothing).to_vector . should_equal nulls + Test.specify "should handle nulls correctly in not" <| + t = table_builder [["A", [True, False, Nothing]]] + a = t.at "A" + a_not = a.not + a_not.to_vector . should_equal [False, True, Nothing] + + Test.specify "should handle nulls correctly in &&" <| + t = table_builder [["A", [True, True, True, False, False, False, Nothing, Nothing, Nothing]], ["B", [True, False, Nothing, True, False, Nothing, True, False, Nothing]]] + a = t.at "A" + (a && True).to_vector . should_equal [True, True, True, False, False, False, Nothing, Nothing, Nothing] + (a && False).to_vector . should_equal [False, False, False, False, False, False, False, False, False] + (a && Nothing).to_vector . should_equal [Nothing, Nothing, Nothing, False, False, False, Nothing, Nothing, Nothing] + + a_not = a.not + (a_not && True).to_vector . should_equal [False, False, False, True, True, True, Nothing, Nothing, Nothing] + (a_not && False).to_vector . should_equal [False, False, False, False, False, False, False, False, False] + (a_not && Nothing).to_vector . should_equal [False, False, False, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing] + + b = t.at "B" + (a && b).to_vector . should_equal [True, False, Nothing, False, False, False, Nothing, False, Nothing] + (a_not && b).to_vector . should_equal [False, False, False, True, False, Nothing, Nothing, False, Nothing] + + Test.specify "should handle nulls correctly in ||" <| + t = table_builder [["A", [True, True, True, False, False, False, Nothing, Nothing, Nothing]], ["B", [True, False, Nothing, True, False, Nothing, True, False, Nothing]]] + a = t.at "A" + (a || True).to_vector . should_equal [True, True, True, True, True, True, True, True, True] + (a || False).to_vector . should_equal [True, True, True, False, False, False, Nothing, Nothing, Nothing] + (a || Nothing).to_vector . should_equal [True, True, True, Nothing, Nothing, Nothing, Nothing, Nothing, Nothing] + + a_not = a.not + (a_not || True).to_vector . should_equal [True, True, True, True, True, True, True, True, True] + (a_not || False).to_vector . should_equal [False, False, False, True, True, True, Nothing, Nothing, Nothing] + (a_not || Nothing).to_vector . should_equal [Nothing, Nothing, Nothing, True, True, True, Nothing, Nothing, Nothing] + + b = t.at "B" + (a || b).to_vector . should_equal [True, True, True, True, False, Nothing, True, Nothing, Nothing] + (a_not || b).to_vector . should_equal [True, False, Nothing, True, True, True, True, Nothing, Nothing] Test.specify "should check types" <| t = table_builder [["X", [1, 2, 3]], ["Y", ['a', 'b', 'c']], ["Z", [True, False, Nothing]]] diff --git a/test/Table_Tests/src/In_Memory/Table_Spec.enso b/test/Table_Tests/src/In_Memory/Table_Spec.enso index 5b96fa6ccc0..1de5b986bcc 100644 --- a/test/Table_Tests/src/In_Memory/Table_Spec.enso +++ b/test/Table_Tests/src/In_Memory/Table_Spec.enso @@ -233,7 +233,7 @@ spec = gt_b = a > b gt_b.to_vector.should_equal [False, False, True, Nothing, False] both = gt_const && gt_b - both.to_vector.should_equal [False, False, True, Nothing, False] + both.to_vector.should_equal [False, False, True, False, False] Test.specify "should handle Text operations" <| a = Column.from_vector 'a' ["abab", "abc", Nothing, "bca", "acca"]