Skip to content

Commit

Permalink
Merge pull request #36 from cheerfulstoic/add-errors-for-long-postgre…
Browse files Browse the repository at this point in the history
…s-identifiers

fix: Add errors for when postgres identifiers would be too long
  • Loading branch information
cheerfulstoic authored Feb 13, 2025
2 parents 3526724 + d871bb4 commit c8f3092
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 1 deletion.
43 changes: 42 additions & 1 deletion lib/ecto_watch/watcher_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ defmodule EctoWatch.WatcherServer do
[options.schema_definition.primary_key | options.extra_columns]
|> Enum.map_join(",", &"'#{&1}',row.#{&1}")

details = watcher_details(%{unique_label: unique_label, repo_mod: repo_mod, options: options})
details =
watcher_details(%{unique_label: unique_label, repo_mod: repo_mod, options: options})

validate_watcher_details!(details, options)

Ecto.Adapters.SQL.query!(
repo_mod,
Expand All @@ -92,6 +95,7 @@ defmodule EctoWatch.WatcherServer do
)

# Can't use the "OR REPLACE" syntax before postgres v13.3.4, so using DROP TRIGGER IF EXISTS
# THOUGHT: Could messages be lost during the drop/re-create?
Ecto.Adapters.SQL.query!(
repo_mod,
"""
Expand Down Expand Up @@ -238,6 +242,43 @@ defmodule EctoWatch.WatcherServer do
}
end

defp validate_watcher_details!(watcher_details, watcher_options) do
case Ecto.Adapters.SQL.query!(watcher_details.repo_mod, "SHOW max_identifier_length", []) do
%Postgrex.Result{rows: [[max_identifier_length]]} ->
max_identifier_length = String.to_integer(max_identifier_length)

max_byte_size =
max(
byte_size(watcher_details.function_name),
byte_size(watcher_details.trigger_name)
)

if max_byte_size > max_identifier_length do
difference = max_byte_size - max_identifier_length

if watcher_options.label do
raise """
Error for watcher: #{inspect(identifier(watcher_options))}
Label is #{difference} character(s) too long for the auto-generated Postgres trigger name.
"""
else
raise """
Error for watcher: #{inspect(identifier(watcher_options))}
Schema module name is #{difference} character(s) too long for the auto-generated Postgres trigger name.
You may want to use the `label` option
"""
end
end

other ->
raise "Unexpected result when querying for max_identifier_length: #{inspect(other)}"
end
end

def topics(update_type, unique_label, returned_values, identifier_columns)
when update_type in ~w[inserted updated deleted]a do
[
Expand Down
118 changes: 118 additions & 0 deletions test/ecto_watch_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,30 @@ defmodule EctoWatchTest do
end
end

defmodule ModuleWithAReallyLongName do
use Ecto.Schema

@moduledoc """
A module that *just* barely fits when creating function/trigger names
"""

schema "a_module_with_a_really_long_name" do
field(:the_string, :string)
end
end

defmodule ModuleWithJustTooLongAName do
use Ecto.Schema

@moduledoc """
A module that *just* barely fits when creating function/trigger names
"""

schema "a_module_with_just_too_long_a_name" do
field(:the_string, :string)
end
end

setup do
start_supervised!(TestRepo)

Expand Down Expand Up @@ -91,6 +115,28 @@ defmodule EctoWatchTest do
[]
)

Ecto.Adapters.SQL.query!(
TestRepo,
"""
CREATE TABLE a_module_with_a_really_long_name (
id SERIAL PRIMARY KEY,
the_string TEXT
)
""",
[]
)

Ecto.Adapters.SQL.query!(
TestRepo,
"""
CREATE TABLE a_module_with_just_too_long_a_name (
id SERIAL PRIMARY KEY,
the_string TEXT
)
""",
[]
)

%Postgrex.Result{
rows: [[already_existing_id1]]
} =
Expand Down Expand Up @@ -486,6 +532,78 @@ defmodule EctoWatchTest do
start_supervised!({EctoWatch, repo: TestRepo, pub_sub: TestPubSub, watchers: []})
end

test "Errors should be given if the schema module is too long for creating the trigger name" do
assert_raise RuntimeError,
~r/Schema module name is 1 character\(s\) too long for the auto-generated Postgres trigger/,
fn ->
start_supervised!(
{EctoWatch,
repo: TestRepo,
pub_sub: TestPubSub,
watchers: [
{ModuleWithJustTooLongAName, :inserted}
]}
)
end

# Everything works fine if you're just at the limit
start_supervised!(
{EctoWatch,
repo: TestRepo,
pub_sub: TestPubSub,
watchers: [
{ModuleWithAReallyLongName, :inserted}
]}
)

EctoWatch.subscribe({ModuleWithAReallyLongName, :inserted})

Ecto.Adapters.SQL.query!(
TestRepo,
"INSERT INTO a_module_with_a_really_long_name (the_string) VALUES ('the value')",
[]
)

assert_receive {{ModuleWithAReallyLongName, :inserted}, %{id: _}}
end

test "Errors should be given if the label is too long" do
assert_raise RuntimeError,
~r/Label is 1 character\(s\) too long for the auto-generated Postgres trigger name/,
fn ->
start_supervised!(
{EctoWatch,
repo: TestRepo,
pub_sub: TestPubSub,
watchers: [
{ModuleWithJustTooLongAName, :inserted,
label: :the_label_is_also_just_much_too_long_such_a_shame}
]}
)
end

# Everything works fine if you're just at the limit
start_supervised!(
{EctoWatch,
repo: TestRepo,
pub_sub: TestPubSub,
watchers: [
{ModuleWithJustTooLongAName, :inserted,
label: :the_label_is_also_just_much_too_long_such_a_sham}
]}
)

EctoWatch.subscribe(:the_label_is_also_just_much_too_long_such_a_sham)

Ecto.Adapters.SQL.query!(
TestRepo,
"INSERT INTO a_module_with_just_too_long_a_name (the_string) VALUES ('the value')",
[]
)

assert_receive {:the_label_is_also_just_much_too_long_such_a_sham, %{id: _}}
end

test "subscribe requires proper Ecto schema", %{
already_existing_id1: already_existing_id1
} do
Expand Down

0 comments on commit c8f3092

Please sign in to comment.