Repository: polarsignals/frostdb (go)

Issue:
Project null column if selecting column that doesn't exist
We need this for https://github.com/parca-dev/parca/pull/2667

So we can write a query like this:

```
labels.a == "test" AND pprof_labels.a IS NULL
or
labels.a is NULL AND pprof_labels.a == "test"
```

The problem is that this doesn't work currently since projections don't return a column if it doesn't actually exist.

Pull request:
394 project null columns
Closes #394 

So I don't think we actually need to project the null column here to solve this problem, we just actually needed to handle the missing column case during binary scalar expression evaluations. 

Patch:
diff --git a/binaryscalarexpr.go b/binaryscalarexpr.go
index c3c4020..a1206bd 100644
--- a/binaryscalarexpr.go
+++ b/binaryscalarexpr.go
@@ -49,6 +49,15 @@ func (e BinaryScalarExpr) Eval(p Particulate) (bool, error) {
 	// existant columns or null values. I'm pretty sure this is completely
 	// wrong and needs per operation, per type specific behavior.
 	if !exists {
+		if e.Right.IsNull() {
+			switch e.Op {
+			case logicalplan.OpEq:
+				return true, nil
+			case logicalplan.OpNotEq:
+				return false, nil
+			}
+		}
+
 		// only handling string for now.
 		if e.Right.Kind() == parquet.ByteArray || e.Right.Kind() == parquet.FixedLenByteArray {
 			switch {
@@ -69,7 +78,7 @@ var ErrUnsupportedBinaryOperation = errors.New("unsupported binary operation")
 func BinaryScalarOperation(left parquet.ColumnChunk, right parquet.Value, operator logicalplan.Op) (bool, error) {
 	switch operator {
 	case logicalplan.OpEq:
-		if right == parquet.NullValue() {
+		if right.IsNull() {
 			// Assume all ColumnChunk have NULLs for now.
 			// They will be read and added to a bitmap later on.
 			// TODO: Maybe there's a nice way of reading the NumNulls from the Pages, for me they always return 0
diff --git a/query/physicalplan/binaryscalarexpr.go b/query/physicalplan/binaryscalarexpr.go
index be3dad9..e5bfebc 100644
--- a/query/physicalplan/binaryscalarexpr.go
+++ b/query/physicalplan/binaryscalarexpr.go
@@ -41,14 +41,29 @@ func (e BinaryScalarExpr) Eval(r arrow.Record) (*Bitmap, error) {
 		return nil, err
 	}
 
-	// TODO: This needs a bunch of test cases to validate edge cases like non
-	// existant columns or null values. I'm pretty sure this is completely
-	// wrong and needs per operation, per type specific behavior.
 	if !exists {
 		res := NewBitmap()
-		for i := uint32(0); i < uint32(r.NumRows()); i++ {
-			res.Add(i)
+		switch e.Op {
+		case logicalplan.OpEq:
+			if e.Right.IsValid() { // missing column; looking for == non-nil
+				switch t := e.Right.(type) {
+				case *scalar.Binary:
+					if t.String() != "" { // treat empty string equivalent to nil
+						return res, nil
+					}
+				case *scalar.String:
+					if t.String() != "" { // treat empty string equivalent to nil
+						return res, nil
+					}
+				}
+			}
+		case logicalplan.OpNotEq: // missing column; looking for != nil
+			if !e.Right.IsValid() {
+				return res, nil
+			}
 		}
+
+		res.AddRange(0, uint64(r.NumRows()))
 		return res, nil
 	}
 


Test patch:
diff --git a/logictest/testdata/exec/filter/filter_projection b/logictest/testdata/exec/filter/filter_projection
index ced281d..52bb0ab 100644
--- a/logictest/testdata/exec/filter/filter_projection
+++ b/logictest/testdata/exec/filter/filter_projection
@@ -20,3 +20,20 @@ select timestamp where timestamp >= 2
 ----
 2
 3
+
+# projection of null column
+exec
+select value where labels.label5 = null and labels.label3 != null
+----
+2
+
+# inverse projection of null column
+exec
+select value where labels.label5 != null and labels.label3 != null
+----
+
+# multi null column projection
+exec
+select value where (labels.label3 = 'value3' and labels.label5 = null) or (labels.label3 = null and labels.label5 = 'a')
+----
+2