fix: Add special case handling for comparing empty and nil values (#19348)

We can have a predicate checking for an empty string, e.g. {"foo" = ""}. If a row has no value for the column "foo", it will return a nil value for that column. Thus, when we go to compare the row with the predicate, we will be comparing an empty value to a nil value. This change adjusts the comparison check so that we correctly handle this special case.

pkg/dataobj/internal/dataset/value.go

@@ -269,8 +269,16 @@ func CompareValues(a, b *Value) int {
 	// Handle nil values first to avoid the panic if the types don't match.
 	switch {
 	case aNil && !bNil:
+		if bType == datasetmd.PHYSICAL_TYPE_BINARY && b.IsZero() {
+			// Nil value for a and empty string for b should still be treated as equal
+			return 0
+		}
 		return -1
 	case !aNil && bNil:
+		if aType == datasetmd.PHYSICAL_TYPE_BINARY && a.IsZero() {
+			// Empty string for a and nil value for b should still be treated as equal
+			return 0
+		}
 		return 1
 	case aNil && bNil:
 		return 0

pkg/dataobj/internal/dataset/value_test.go

@@ -102,6 +102,30 @@ func BenchmarkCompareValues(b *testing.B) {
 	}
 }
 
+func TestEmptyNil_CompareValues(t *testing.T) {
+	t.Run("Empty vs empty", func(t *testing.T) {
+		a := dataset.BinaryValue([]byte{})
+		b := dataset.BinaryValue([]byte{})
+
+		require.Equal(t, dataset.CompareValues(&a, &b), 0)
+	})
+
+	t.Run("Nil vs empty", func(t *testing.T) {
+		var a dataset.Value
+		b := dataset.BinaryValue([]byte{})
+
+		require.Equal(t, dataset.CompareValues(&a, &b), 0)
+		require.Equal(t, dataset.CompareValues(&b, &a), 0)
+	})
+	t.Run("Nil vs nil", func(t *testing.T) {
+		var a dataset.Value
+		var b dataset.Value
+
+		require.Equal(t, dataset.CompareValues(&a, &b), 0)
+		require.Equal(t, dataset.CompareValues(&b, &a), 0)
+	})
+}
+
 func TestValue_MarshalBinary(t *testing.T) {
 	t.Run("Null", func(t *testing.T) {
 		var expect dataset.Value