Skip to content

fix(sync-service): array cast assigns element type instead of array type#4348

Merged
robacourt merged 3 commits into
mainfrom
rob/fix-inclusion
May 19, 2026
Merged

fix(sync-service): array cast assigns element type instead of array type#4348
robacourt merged 3 commits into
mainfrom
rob/fix-inclusion

Conversation

@robacourt
Copy link
Copy Markdown
Contributor

@robacourt robacourt commented May 18, 2026

Summary

  • Where clauses like "organization_ids" @> ARRAY[$1]::uuid[] return HTTP 400 (Could not select an operator overload) whenever the inner ARRAY[...] constructor contains a non-foldable element (a column ref, or a parameter inside a sub-expression that the parser can't reduce ahead of the outer cast).
  • Root cause is in Electric.Replication.Eval.Parser.as_dynamic_cast/3: the :array_cast branch built a Func with type: extract_base_type_name(target_type) plus map_over_array_in_pos: 0. So for target {:array, :uuid} it stored .type = :uuid while the runtime value is still an array — and the operator-overload lookup (Lookups.pick_concrete_operator_overload/3) reads .type literally, can't match anyarray @> anyarray, and the API surfaces a 400. cast_implicit/3 and the ANY/ALL builder set the same misleading scalar type, and try_applying/1 had a wrap-in-{:array, _} kludge at line 1582 to paper over it.

This PR changes the convention so Func.type always reflects the runtime type of the value the Func produces:

  • as_dynamic_cast/3 :array_cast branch now keeps the full target array type.
  • cast_implicit/3 array branch now stores {:array, to_type}.
  • handle_any_or_all/2's bool_array now stores {:array, :bool}.
  • try_applying/1 drops the conditional wrap.

The bug stayed hidden for the all-constant case because do_maybe_reduce/1 folds ARRAY[Const, ...] into a Const{type: {:array, T}, value: [...]} before the outer cast runs; the parser test added in this PR uses a column ref to keep the Array unreduced and exposes the mis-typed Func directly.

Test plan

  • mix test packages/sync-service/test/electric/replication/eval/parser_test.exs — 85 tests, including the new minimal assertion at parser_test.exs:1092
  • mix test packages/sync-service/test/electric/replication/eval/ — 227 tests + 6 properties + 3 doctests, 0 failures
  • mix test packages/sync-service/test/electric/shapes/filter_test.exs — 135 tests, 0 failures (covers array_field @> const_array inclusion-index optimisation)
  • mix test packages/sync-service/test/electric/shapes/querying_test.exs — 28 tests, 0 failures
  • CI green

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.99%. Comparing base (ed48bba) to head (fcbd798).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4348      +/-   ##
==========================================
- Coverage   59.19%   58.99%   -0.20%     
==========================================
  Files         305      290      -15     
  Lines       28415    27871     -544     
  Branches     7495     7496       +1     
==========================================
- Hits        16819    16442     -377     
+ Misses      11582    11415     -167     
  Partials       14       14              
Flag Coverage Δ
electric-telemetry ?
elixir ?
packages/agents 70.47% <ø> (ø)
packages/agents-mcp 77.54% <ø> (ø)
packages/agents-runtime 80.03% <ø> (+0.06%) ⬆️
packages/agents-server 73.15% <ø> (-0.31%) ⬇️
packages/agents-server-ui 6.04% <ø> (ø)
packages/electric-ax 42.38% <ø> (ø)
packages/experimental 87.73% <ø> (ø)
packages/react-hooks 86.48% <ø> (ø)
packages/start 82.83% <ø> (ø)
packages/typescript-client 94.39% <ø> (-0.06%) ⬇️
packages/y-electric 56.05% <ø> (ø)
typescript 58.99% <ø> (-0.04%) ⬇️
unit-tests 58.99% <ø> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@alco alco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@robacourt robacourt merged commit 8a94d7f into main May 19, 2026
68 checks passed
@robacourt robacourt deleted the rob/fix-inclusion branch May 19, 2026 08:37
@robacourt robacourt self-assigned this May 19, 2026
@github-actions
Copy link
Copy Markdown
Contributor

This PR has been released! 🚀

The following packages include changes from this PR:

  • @core/sync-service@1.6.6

Thanks for contributing to Electric!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants