cheerfulstoic / ecto_watch

EctoWatch allows you to easily get notifications about database changes directly from PostgreSQL.
93 stars 2 forks source link

[Question] Schema prefix support #1

Open cjimison opened 1 week ago

cjimison commented 1 week ago

Real cool project however I have a question about prefix support.

I have some tables that are shared across multiple microservices in my cluster and some of those are not ecto (or elixir) based. This would be an AMAZING project to help notify my elixir app when another microservice modifies a table. However these other systems don't always do things the "Ecto" way in particular when it comes to using Postgres Prefixes.

I normally get around this by doing something like so:

defmodule SomeSchema do
  use Ecto.Schema

  @schema_prefix "some_prefix"
  @primary_key {:private_key_name, :binary, autogenerate: false}
  schema "some_table" do
    field :some_field, :integer
    ...
  end
end

However when I try to hook EctoWatch up to this it complains about the table does not exist but I can see it in PGAdmin and my other code can read from it just fine.

So does ecto_watch not support prefixes or am I doing something wrong? If so do you have any documentation or pointers for how I can set this up?

Thanks!

cjimison commented 1 week ago

Note: I was able to get this to stop the error (but no process callbacks yet) if I added a after_connect callback option on my config and set the search path like so:

Postgrex.query(conn, "SET search_path=#{path}", [])

Here is a post that might help other readers on setting the search path. It is A solution but it would be great if it would also load the @schema_prefix as well.

cheerfulstoic commented 1 week ago

Yeah, I didn't even think about prefix. I think I've seen them before, but I've never used them. I'll look at adding this now 👍

cheerfulstoic commented 1 week ago

Ok, I added some tests and made a fix to support the @schema_prefix, so maybe give 0.5.0 a try and let me know if it works or not?

cjimison commented 1 week ago

I don't think that code works for me because it is just passing in the raw prefix in to the string without tokenizing/parsing it. For example:

I work with blockchains and this other library creates a prefix based on the contract address and caches the data. So this means my prefixes look like so:

@schema_prefix "0x8d8b6b8414e1e3dcfd4168561b9be6bd3bf6ec4b"

I am not arguing that this is a good idea .. in fact I wish they hadn't done that, but what is done is done. However when launch my app I am now getting this error:

** (Mix) Could not start application mud: MUD.Application.start(:normal, []) returned an
 error: shutdown: failed to start child: EctoWatch
    ** (EXIT) shutdown: failed to start child: EctoWatch.WatcherSupervisor
        ** (EXIT) shutdown: failed to start child: :ew_inserted_for_mud_data_notification
            ** (EXIT) an exception was raised:
                ** (Postgrex.Error) ERROR 42601 (syntax_error) syntax error at or near "0x8d8b6b8414e1e3dcfd4168561b9be6bd3bf6ec4bg"

    query: CREATE OR REPLACE TRIGGER ew_inserted_for_mud_data_notification_trigger
  AFTER INSERT ON 0x8d8b6b8414e1e3dcfd4168561b9be6bd3bf6ec4bapp__notification_table FOR
 EACH ROW
  EXECUTE PROCEDURE ew_inserted_for_mud_data_notification_func();

                    (ecto_sql 3.11.3) lib/ecto/adapters/sql.ex:1054: Ecto.Adapters.SQL.r
aise_sql_call_error/1
                    (ecto_watch 0.5.0) lib/ecto_watch/watcher_server.ex:89: EctoWatch.Wa
tcherServer.init/1
                    (stdlib 6.0) gen_server.erl:2057: :gen_server.init_it/2
                    (stdlib 6.0) gen_server.erl:2012: :gen_server.init_it/6
                    (stdlib 6.0) proc_lib.erl:329: :proc_lib.init_p_do_apply/3

so I wonder if the string argument is not getting tokenized correctly so that it can safely be passed into postgres for execution.

cheerfulstoic commented 1 week ago

Ah, sure 🤔 Or I wonder if maybe there need to be quotes around the table... Going to use your example in the test and see what happens...

cjimison commented 1 week ago

Also I wonder if it would be better to allow the prefix to be part of the server options passed in. That way you can have a rolling set of defaults like what ecto uses.

If watcher_opts.prefix is nil, then use the modules prefix as a default.

cheerfulstoic commented 1 week ago

I just released 0.5.1 which uses quotes which I think should allow for special characters...

Could you give an example for giving a prefix server option? I hoped to be able to depend on the schema definition, but I see that you can specify prefixes on a per-query basis, so obviously that's not always happening. Is that what you mean?

Thanks!

cjimison commented 1 week ago

Hmm.. I am now getting a Postgrex.Error) ERROR 42P01 (undefined_table) relation. Looking at your test code I don't think you are actually creating a schema here, just prepending to the table name.

When I look at my PGAdmin console after running your test I see the following:

image

So according to Postgres you are still using the public schema, just adding some text in front of the table name.

Also I took a look at the postgres documentation here and it says you should be using a dot notation between the schema and the name. However when I tried to add that in myself it still failed with a "undefined table" so not sure.

What I would expect to see is a new schema called "0xabcd" and in it a table named "things" if everything worked correctly.

cjimison commented 1 week ago

I created a quick and dirty PR that I think works. Give it a shot and let me know.

https://github.com/cheerfulstoic/ecto_watch/pull/2

cjimison commented 1 week ago

Also I don't think this is actually working. In the unit tests because the pubsub doesn't take the schema into account and the way the triggers look like they are setup for the "0xabcd" based objects:

image

cjimison commented 1 week ago

The PR has been updated to at least put the triggers into the right schema, and the unit tests pass, however when I use this in my main project I am not getting any callbacks on the EctoWatch.WatcherServer so I am not sure if that code actually works.

I might pick this back up in the next day or so. Thanks for the help and again really cool project!

cheerfulstoic commented 6 days ago

Aaaahh, ok! I was misunderstanding (and probably not reading closely enough 😅). I think part of the confusion comes from the fact that the term "schema" refers to two things here: The Ecto module which represents a table and the context/namespace in the DB where tables exist (at least in Postgres. I guess in mySQL they are called "databases", but we're not worrying about mySQL for now (or ever?))

So I can see why the attribute is @schema_prefix instead of just @prefix. I thought it was a way to namespace tables within a postgres schema (mainly because I wasn't even thinking about postgres schemas)

Merged your PR and made a couple of small tweaks. Since you said it wasn't working for you yet I won't make a release just yet, but I might play around some too.

cheerfulstoic commented 5 days ago

I just released this change as 0.5.2 because I realized that while it might not be perfect, it's better than 0.5.1 😅