Fix non subquery conditions in tags

Author: robacourt

packages/sync-service/lib/electric/shapes/querying.ex

@@ -4,6 +4,7 @@ defmodule Electric.Shapes.Querying do
   alias Electric.Shapes.Shape
   alias Electric.Shapes.Shape.SubqueryMoves
   alias Electric.Replication.Eval.Decomposer
+  alias Electric.Replication.Eval.Parser.{Const, Ref, Func, Array}
   alias Electric.Telemetry.OpenTelemetry
 
   @value_prefix SubqueryMoves.value_prefix()
@@ -333,11 +334,102 @@ defmodule Electric.Shapes.Querying do
   end
 
   # Generate SQL for a non-subquery condition
-  # This is complex as we'd need to convert the AST back to SQL
-  # For now, return true (since the row is in the result, the condition is satisfied)
-  defp generate_non_subquery_condition_sql(_subexpr) do
-    # Non-subquery conditions in the WHERE clause are already satisfied for returned rows
-    # This is a simplification - for full accuracy we'd need AST-to-SQL conversion
+  # Convert the AST back to SQL for accurate evaluation of OR conditions
+  defp generate_non_subquery_condition_sql(subexpr) do
+    ast_to_sql(subexpr.ast)
+  end
+
+  # Convert AST to SQL - handles common comparison operators
+  # Note: Operator names are stored with surrounding quotes (e.g., "\"=\"" for equals)
+  defp ast_to_sql(%Func{name: "\"=\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} = #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\"<>\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} <> #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\"<\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} < #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\">\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} > #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\"<=\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} <= #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\">=\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} >= #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\"~~\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} LIKE #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "\"~~*\"", args: [left, right]}) do
+    "(#{ast_to_sql(left)} ILIKE #{ast_to_sql(right)})"
+  end
+
+  defp ast_to_sql(%Func{name: "is null", args: [arg]}) do
+    "(#{ast_to_sql(arg)} IS NULL)"
+  end
+
+  defp ast_to_sql(%Func{name: "is not null", args: [arg]}) do
+    "(#{ast_to_sql(arg)} IS NOT NULL)"
+  end
+
+  defp ast_to_sql(%Func{name: "in", args: [left, %Array{elements: elements}]}) do
+    values = Enum.map(elements, &ast_to_sql/1) |> Enum.join(", ")
+    "(#{ast_to_sql(left)} IN (#{values}))"
+  end
+
+  defp ast_to_sql(%Func{name: "not", args: [inner]}) do
+    "(NOT #{ast_to_sql(inner)})"
+  end
+
+  defp ast_to_sql(%Func{name: "and", args: args}) do
+    conditions = Enum.map(args, &ast_to_sql/1) |> Enum.join(" AND ")
+    "(#{conditions})"
+  end
+
+  defp ast_to_sql(%Func{name: "or", args: args}) do
+    conditions = Enum.map(args, &ast_to_sql/1) |> Enum.join(" OR ")
+    "(#{conditions})"
+  end
+
+  defp ast_to_sql(%Ref{path: path}) do
+    # Quote the column name
+    ~s|"#{Enum.join(path, "\".\"")}"|
+  end
+
+  defp ast_to_sql(%Const{value: nil}) do
+    "NULL"
+  end
+
+  defp ast_to_sql(%Const{value: value}) when is_binary(value) do
+    # Escape single quotes in the value
+    escaped = String.replace(value, "'", "''")
+    "'#{escaped}'"
+  end
+
+  defp ast_to_sql(%Const{value: value}) when is_integer(value) or is_float(value) do
+    "#{value}"
+  end
+
+  defp ast_to_sql(%Const{value: true}) do
+    "true"
+  end
+
+  defp ast_to_sql(%Const{value: false}) do
+    "false"
+  end
+
+  defp ast_to_sql(_other) do
+    # Fallback for unsupported AST nodes - return true to avoid breaking
+    # This is conservative: if we can't convert, assume the condition could be satisfied
     "true"
   end
 

packages/sync-service/test/integration/active_conditions_test.exs

@@ -430,6 +430,9 @@ defmodule Electric.Integration.ActiveConditionsTest do
         deleted_ids = Enum.map(deletes, & &1.value["id"]) |> Enum.sort()
         assert deleted_ids == ["child-1", "child-3"]
 
+        # Consume the up_to_date that follows the move-out processing
+        assert_up_to_date(consumer)
+
         # Verify child-2 was NOT deleted by checking we can still update it
         Postgrex.query!(
           db_conn,