From 43b130d39f972d426afe777b86b6d7e60f7aae5d Mon Sep 17 00:00:00 2001
From: Milan Bouchet-Valat <nalimilan@club.fr>
Date: Wed, 27 Sep 2017 18:23:16 +0200
Subject: [PATCH] Change comparison operators to return null when one of the
 operands is null

This is consistent with DataArrays, SQL and R, is more consistent with
the null-propagating behavior used with other operations, and is safer
in case null values were not expected (as an error will be thrown).

isequal() and isless() still return true or false.
---
 src/Nulls.jl     | 24 +++++++++++++++++-------
 test/runtests.jl | 43 ++++++++++++++++++++++++-------------------
 2 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/src/Nulls.jl b/src/Nulls.jl
index 9b094b4..7a82e05 100644
--- a/src/Nulls.jl
+++ b/src/Nulls.jl
@@ -1,7 +1,7 @@
 __precompile__(true)
 module Nulls
 
-import Base: *, <, ==, <=, +, -, ^, /, &, |, xor
+import Base: *, <, ==, !=, <=, +, -, ^, /, &, |, xor
 
 export null, nulls, Null
 
@@ -37,16 +37,26 @@ Base.promote_rule(::Type{T}, ::Type{Null}) where {T} = Union{T, Null}
 Base.convert(::Type{Union{T, Null}}, x) where {T} = convert(T, x)
 
 # Comparison operators
-<(::Null, ::Null) = false
-<(::Null, b) = false
-<(a, ::Null) = false
+==(::Null, ::Null) = null
+==(::Null, b) = null
+==(a, ::Null) = null
+# != must be defined explicitly since fallback expects a Bool
+!=(::Null, ::Null) = null
+!=(::Null, b) = null
+!=(a, ::Null) = null
+Base.isequal(::Null, ::Null) = true
+Base.isequal(::Null, b) = false
+Base.isequal(a, ::Null) = false
+<(::Null, ::Null) = null
+<(::Null, b) = null
+<(a, ::Null) = null
 Base.isless(::Null, ::Null) = false
 Base.isless(::Null, b) = false
 Base.isless(a, ::Null) = true
 if VERSION < v"0.7.0-DEV.300"
-    <=(::Null, ::Null) = true
-    <=(::Null, b) = false
-    <=(a, ::Null) = false
+    <=(::Null, ::Null) = null
+    <=(::Null, b) = null
+    <=(a, ::Null) = null
 end
 
 # Unary operators/functions
diff --git a/test/runtests.jl b/test/runtests.jl
index 33433d6..6757903 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -35,18 +35,21 @@ using Base.Test, Nulls
     end
 
     # Comparison operators
-    @test null == null
-    @test !(1 == null)
-    @test !(null == 1)
-    @test !(null != null)
-    @test 1 != null
-    @test null != 1
-    @test !(null < null)
-    @test !(null < 1)
-    @test !(1 < null)
-    @test null <= null
-    @test !(null <= 1)
-    @test !(1 <= null)
+    @test (null == null) === null
+    @test (1 == null) === null
+    @test (null == 1) === null
+    @test (null != null) === null
+    @test (1 != null) === null
+    @test (null != 1) === null
+    @test isequal(null, null)
+    @test !isequal(1, null)
+    @test !isequal(null, 1)
+    @test (null < null) === null
+    @test (null < 1) === null
+    @test (1 < null) === null
+    @test (null <= null) === null
+    @test (null <= 1) === null
+    @test (1 <= null) === null
     @test !isless(null, null)
     @test !isless(null, 1)
     @test isless(1, null)
@@ -83,7 +86,7 @@ using Base.Test, Nulls
     @test size(null, 1) == 1
     @test_throws BoundsError size(null, 0)
     @test ndims(null) == 0
-    @test null[1] == null
+    @test null[1] === null
     @test_throws BoundsError null[2]
 
     @test eltype([1, 2, null]) == Union{Int, Null}
@@ -114,25 +117,27 @@ using Base.Test, Nulls
     @test Nulls.coalesce.([null, 1, null], 0) isa Vector{Int}
     @test Nulls.coalesce.([null, 1, null], [0, 10, 5]) == [0, 1, 5]
     @test Nulls.coalesce.([null, 1, null], [0, 10, 5]) isa Vector{Int}
-    @test Nulls.coalesce.([null, 1, null], [0, null, null]) == [0, 1, null]
+    @test isequal(Nulls.coalesce.([null, 1, null], [0, null, null]), [0, 1, null])
     # Fails in Julia 0.6 and 0.7.0-DEV.1556
     @test_broken Nulls.coalesce.([null, 1, null], [0, null, null]) isa Vector{Union{Null, Int}}
 
     x = convert(Vector{Union{Int, Null}}, [1.0, null])
     @test isa(x, Vector{Union{Int, Null}})
-    @test x == [1, null]
+    @test isequal(x, [1, null])
     x = convert(Vector{Union{Int, Null}}, [1.0])
     @test isa(x, Vector{Union{Int, Null}})
     @test x == [1]
     x = convert(Vector{Union{Int, Null}}, [null])
     @test isa(x, Vector{Union{Int, Null}})
-    @test x == [null]
+    @test isequal(x, [null])
 
     @test Nulls.T(Union{Int, Null}) == Int
 
-    @test nulls(1) == [null]
-    @test nulls(Int, 1) == (Union{Int, Null})[null]
-    @test nulls(Union{Int, Null}, 1, 2) == (Union{Int, Null})[null null]
+    @test isequal(nulls(1), [null])
+    @test isequal(nulls(Int, 1), [null])
+    @test nulls(Int, 1) isa Vector{Union{Int, Null}}
+    @test isequal(nulls(Union{Int, Null}, 1, 2), [null null])
+    @test nulls(Union{Int, Null}, 1, 2) isa Matrix{Union{Int, Null}}
     @test Union{Int, Null}[1,2,3] == (Union{Int, Null})[1,2,3]
 
     @test convert(Union{Int, Null}, 1.0) == 1