chore: fix: RHS null filter and add RHS NULL filter support for postg…

…res (#208)
hypertrace · Aug 30, 2024 · d5362f6 · d5362f6
1 parent d96a4a3
commit d5362f6
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 7 deletions.
diff --git a/...store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java b/...store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java
@@ -844,6 +844,9 @@ void testAggregateWithNestedArraysAndUnnestFilters(final String dataStoreName)
     final FilterTypeExpression filter =
         LogicalExpression.builder()
             .operator(AND)
+            .operand(
+                RelationalExpression.of(
+                    IdentifierExpression.of("item"), NEQ, ConstantExpression.of((String) null)))
             .operand(
                 RelationalExpression.of(
                     IdentifierExpression.of("sales.medium.type"),

diff --git a/...e/documentstore/mongo/query/parser/filter/MongoStandardNonExprRelationalFilterParser.java b/...e/documentstore/mongo/query/parser/filter/MongoStandardNonExprRelationalFilterParser.java
@@ -1,5 +1,8 @@
 package org.hypertrace.core.documentstore.mongo.query.parser.filter;
 
+import static java.util.Collections.unmodifiableMap;
+
+import java.util.HashMap;
 import java.util.Map;
 import lombok.AllArgsConstructor;
 import org.hypertrace.core.documentstore.expression.impl.RelationalExpression;
@@ -16,6 +19,11 @@ public Map<String, Object> parse(
     final String parsedLhs = expression.getLhs().accept(context.lhsParser());
     final String operator = mapping.getOperator(expression.getOperator());
     final Object parsedRhs = expression.getRhs().accept(context.rhsParser());
-    return Map.of(parsedLhs, Map.of(operator, parsedRhs));
+
+    // using HashMap instead of Map.of() as RHS value can be null
+    // but Map.of() doesn't support null values
+    final Map<String, Object> operatorToRhsMap = new HashMap<>();
+    operatorToRhsMap.put(operator, parsedRhs);
+    return Map.of(parsedLhs, unmodifiableMap(operatorToRhsMap));
   }
 }
diff --git a/...documentstore/postgres/query/v1/parser/filter/PostgresStandardRelationalFilterParser.java b/...documentstore/postgres/query/v1/parser/filter/PostgresStandardRelationalFilterParser.java
@@ -10,10 +10,14 @@ class PostgresStandardRelationalFilterParser implements PostgresRelationalFilter
   public String parse(
       final RelationalExpression expression, final PostgresRelationalFilterContext context) {
     final Object parsedLhs = expression.getLhs().accept(context.lhsParser());
-    final String operator = mapper.getMapping(expression.getOperator());
     final Object parsedRhs = expression.getRhs().accept(context.rhsParser());
+    final String operator = mapper.getMapping(expression.getOperator(), parsedRhs);
 
-    context.getParamsBuilder().addObjectParam(parsedRhs);
-    return String.format("%s %s ?", parsedLhs, operator);
+    if (parsedRhs != null) {
+      context.getParamsBuilder().addObjectParam(parsedRhs);
+      return String.format("%s %s ?", parsedLhs, operator);
+    } else {
+      return String.format("%s %s NULL", parsedLhs, operator);
+    }
   }
 }
diff --git a/...cumentstore/postgres/query/v1/parser/filter/PostgresStandardRelationalOperatorMapper.java b/...cumentstore/postgres/query/v1/parser/filter/PostgresStandardRelationalOperatorMapper.java
@@ -10,6 +10,7 @@
 
 import com.google.common.collect.Maps;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import org.hypertrace.core.documentstore.expression.operators.RelationalOperator;
 
@@ -24,8 +25,18 @@ class PostgresStandardRelationalOperatorMapper {
               entry(GTE, ">="),
               entry(LTE, "<=")));
 
-  String getMapping(final RelationalOperator operator) {
-    return Optional.ofNullable(mapping.get(operator))
-        .orElseThrow(() -> new UnsupportedOperationException("Unsupported operator: " + operator));
+  private static final Map<RelationalOperator, String> nullRhsOperatorMapping =
+      Maps.immutableEnumMap(Map.ofEntries(entry(EQ, "IS"), entry(NEQ, "IS NOT")));
+
+  String getMapping(final RelationalOperator operator, Object parsedRhs) {
+    if (Objects.nonNull(parsedRhs)) {
+      return Optional.ofNullable(mapping.get(operator))
+          .orElseThrow(
+              () -> new UnsupportedOperationException("Unsupported operator: " + operator));
+    } else {
+      return Optional.ofNullable(nullRhsOperatorMapping.get(operator))
+          .orElseThrow(
+              () -> new UnsupportedOperationException("Unsupported operator: " + operator));
+    }
   }
 }