Matching on table definitions

[cheerfulstoic]

So overall it seems like a bad idea to have users match on a complex object like the table definition map (i.e. the configuration option added in #18)

def handle_info({:ecto_watch, {%{table_name: "comments", primary_key: :comment_id}, _action, _opts}, columns}, socket)
def handle_info({:ecto_watch, {%{table_name: "comments", primary_key: :the_id}, _action, _opts}, columns}, socket)

I gave this example of having different primary keys for the same table in #19, but my point was actually twofold:

• It was partly trying to describe why it's not good to have table definitions as a thing to match on
• It was also to demonstrate why the table definition functionality allows users to shoot themselves in the foot 😅 (because since a table can only have one primary key you'll likely run into weird issues if you do this)

But a better reason, in my mind, is options like columns / association_columns. If you had these two watchers:

    {
      %{
        table_name: "users",
        columns: [:first_name, :last_name, :email],
      }, :updated
    },
    {
      %{
        table_name: "users",
        columns: [:email, :phone_number, :city, :postal_code, :country],
      }, :updated
    },

You would need to disambiguate them be specifying all of the columns in the columns key, in order. And if you changed the columns in the watcher configuration (adding/removing/or shuffling), you need to find all of the handle_infos and fix them. It sounds like an easy thing, but it would be an easy thing to miss if you don't get everything just perfect. It's the sort of thing that seems like it might be handy / a general solution, but it leaves a lot of room for bugs that are easy to miss.

Better, I think, to require creating a label which forces the developer to create a description of what they're trying to do with the watcher:

    {
      %{
        table_name: "users",
        columns: [:first_name, :last_name, :email],
      }, :updated, label: :user_updated_with_identifying_info
    },
    {
      %{
        table_name: "users",
        columns: [:email, :phone_number, :city, :postal_code, :country],
      }, :updated, label: :user_updated_with_contact_details
    },

# and then...
def handle_info({:user_updated_with_identifying_info, data}, socket)
def handle_info({:user_updated_with_contact_details, data}, socket)

[cheerfulstoic]

Just merged #19 and kept the requirement for a label for map table definitions. I talked with a colleague to sanity check the ideas. The biggest thing that I realized today that I hadn't mentioned above: watchers need to provide trigger and function names in postgres. If we use a table_definition to create those names then there's a big question as to how we come up with a name that fits within PostgreSQL's limitations, is unique to the full map's configuration (table_name+primary_key+columns+etc...), *and* to have it be relatively readable. Plus whenever you change any details (esp. the columns/association_columnskeys) then a new trigger/function would get created, leaving behind a trigger/function which still tries to send notifications but which aren't handled. Having alabel` which the trigger/function names can be based on just makes sense in my mind.