diff --git a/.changeset/honest-bees-thank.md b/.changeset/honest-bees-thank.md new file mode 100644 index 0000000000..2e33f0a503 --- /dev/null +++ b/.changeset/honest-bees-thank.md @@ -0,0 +1,5 @@ +--- +'@core/sync-service': patch +--- + +Fix issue with restoring shapes with subqueries which can lead to missing materialzers diff --git a/packages/sync-service/lib/electric/shape_cache/shape_status.ex b/packages/sync-service/lib/electric/shape_cache/shape_status.ex index 9daa1eae73..ab115fd9ed 100644 --- a/packages/sync-service/lib/electric/shape_cache/shape_status.ex +++ b/packages/sync-service/lib/electric/shape_cache/shape_status.ex @@ -483,6 +483,40 @@ defmodule Electric.ShapeCache.ShapeStatus do end) end + defp remove_shapes_with_invalid_dependencies(_stack_id, removed_handles, _storage) + when map_size(removed_handles) == 0 do + :ok + end + + defp remove_shapes_with_invalid_dependencies(stack_id, removed_handles, storage) do + shapes_to_remove = + ShapeDb.reduce_shapes(stack_id, [], fn {handle, shape}, acc -> + if Enum.any?(shape.shape_dependencies_handles, &MapSet.member?(removed_handles, &1)) do + [handle | acc] + else + acc + end + end) + + if shapes_to_remove == [] do + :ok + else + Enum.each(shapes_to_remove, fn handle -> + Logger.warning("Removing shape #{inspect(handle)} because its dependency was invalidated") + + remove_shape(stack_id, handle) + Storage.cleanup!(storage, handle) + end) + + # Recursively check for more shapes that depended on the ones we just removed + remove_shapes_with_invalid_dependencies( + stack_id, + MapSet.new(shapes_to_remove), + storage + ) + end + end + defp load_backup(stack_id, backup_dir, storage) do with :ok <- ShapeDb.restore(stack_id, backup_dir, @backup_version), {:ok, stored_handles} <- Storage.get_all_stored_shape_handles(storage) do @@ -517,6 +551,8 @@ defmodule Electric.ShapeCache.ShapeStatus do Enum.each(invalid_in_memory_handles, fn handle -> remove_shape(stack_id, handle) end) + remove_shapes_with_invalid_dependencies(stack_id, invalid_in_memory_handles, storage) + :ok else {:error, reason} -> diff --git a/packages/sync-service/test/electric/shape_cache/shape_status_test.exs b/packages/sync-service/test/electric/shape_cache/shape_status_test.exs index 16914690dd..54ce1c90fe 100644 --- a/packages/sync-service/test/electric/shape_cache/shape_status_test.exs +++ b/packages/sync-service/test/electric/shape_cache/shape_status_test.exs @@ -436,6 +436,73 @@ defmodule Electric.ShapeCache.ShapeStatusTest do refute File.exists?(backup_dir) end + + test "backup restore removes shapes whose inner shapes were invalidated", %{state: state} do + backup_base_dir = + Path.join(System.tmp_dir!(), "shape_status_test_#{System.unique_integer([:positive])}") + + stub_storage(metadata_backup_dir: fn _ -> backup_base_dir end) + + expect_storage( + get_all_stored_shape_handles: fn _ -> {:ok, MapSet.new()} end, + get_stored_shapes: fn _, _ -> %{} end + ) + + :ok = ShapeStatus.initialize_from_storage(state) + + # Create a shape with a subquery + outer_shape = + %{shape_dependencies: [inner_shape]} = + Shape.new!("public.items", + where: "id IN (SELECT id FROM other_table)", + inspector: @inspector + ) + + {:ok, inner_handle} = ShapeStatus.add_shape(state, inner_shape) + outer_shape = %{outer_shape | shape_dependencies_handles: [inner_handle]} + {:ok, outer_handle} = ShapeStatus.add_shape(state, outer_shape) + + # Also add an independent shape that should survive + independent_shape = shape!("independent") + assert {:ok, independent_handle} = ShapeStatus.add_shape(state, independent_shape) + + # Verify all shapes are present + shapes = ShapeStatus.list_shapes(state) + assert length(shapes) == 3 + + # Save backup with all three shapes + assert :ok = ShapeStatus.save_checkpoint(state) + + backup_dir = Path.join([backup_base_dir, "shape_status_backups"]) + assert File.exists?(backup_dir) + + ShapeStatus.remove(state) + + # Now restore simulating that the outer shape's storage still exists + # but the inner shape's storage has gone + stub_storage([force: true], + metadata_backup_dir: fn _ -> backup_base_dir end, + cleanup!: fn _, _ -> :ok end + ) + + expect_storage([force: true], + get_all_stored_shape_handles: fn _ -> + # Only independent and outer shapes have storage - the inner shape has gone + {:ok, MapSet.new([independent_handle, outer_handle])} + end + ) + + assert :ok = ShapeStatus.initialize_from_storage(state) + + # The outer shape should be removed because its dependency was invalidated + # Only the independent shape should remain + remaining_shapes = ShapeStatus.list_shapes(state) + + assert [{^independent_handle, ^independent_shape}] = remaining_shapes, + "Parent shape with invalid dependency should have been removed. Got: #{inspect(remaining_shapes)}" + + refute File.exists?(backup_dir) + end end defp shape!, do: shape!("test")