Add :constraint_handler and Ecto.Adapters.SQL.Constraint

[petermueller]

• allows setting a :constraint_handler option in locations similar to :prefix
  • config
  • start_link
  • per operation
• adds Ecto.Adapters.SQL.Constraint to replace c:Ecto.Adapters.SQL.Connection.to_constraints/2
• moves existing to_constraints/2 to new modules for the built-in adapters
  • Ecto.Adapters.MyXQL.Constraint
  • Ecto.Adapters.Postgres.Constraint
  • Ecto.Adapters.Tds.Constraint
• still calls the connection's to_constraints/2 for the time being to allow downstream adapters to adopt
• delegates the built-in adapter connections' to_constraints/2 to the new modules
  • e.g. Ecto.Adapters.MyXQL.Connection.to_constraints/2 -> Ecto.Adapters.MyXQL.Constraint.to_constraints/2
• adds tests for the built-in adapters verifying custom constraint handlers work
  • MyXQL
  • Postgres
  • Tds
• adds documentation of how and when this feature is useful and how to use it

[petermueller]

This is a proof of concept PR, from the original thread on the Google Group.

I have not yet swapped the Postgres and Tds implementations, but it should be a matter of a few minutes to do so.

The main things I could use some insights and feedback on are:

1. where should people be allowed to set the :constraint_handler option?
2. how should the "default" constraint handler be set for the built-in adapters?
3. how should Ecto.Adapter.SQL.Connection.to_constraints/2 be deprecated?
4. what do you think of also pulling the Ecto.Adapters.SQL.to_constraints/4 out and making it be a default implementation of doing something like use Ecto.Adapters.SQL.Constraint?

All 4 of these items are somewhat interconnected, and affect the possible options of others.

e.g. doing # 4 would be neat for allowing downstream libraries an escape hatch in case they structure their config differently, but it might make more sense to just put it in the __using__ of Ecto.Adapters.SQL, but that may make it harder to not break downstream things, or hard-deprecate the old behaviour callbacks.

As it is currently written, it should work for downstream libraries without breaking them, but it does not provide a clear path, primarily because I did not want to go down a particular direction without input.

Let me know what you think 🙂
I know this solution would let us more aggressively enforce new/consistent DB controls on a legacy app we're migrating away from while still having a nice developer experience when working w/ Ecto.

[petermueller]

The mysql tests are reliant on an :extra_error_codes value of [{1644, :ER_SIGNAL_EXCEPTION}], and I wasn't quite sure how we'd want to add that in these tests.

I can also just open a PR on myxql to add it, as it's a fairly useful, and likely the error code people would use to write custom errors from within procedures.

[petermueller]

other than moving this migration here, the other changes to this test are from mix format

[petermueller]

I checked through the dependent libraries I could find from hex and ecto_ch (Clickhouse) seemed to be the only one implementing Ecto.Adapters.SQL.Connection, and theirs just raises as "not implemented" currently.

I didn't really see many others that were sticking to the typical patterns from the built-in adapters, but maybe I just missed some.

But I think if we just removed the Connection.to_constraints/2 callback, I don't know that it would cause any issue in downstream adapter libraries

EDIT: I don't know how I missed the SQLite adapter. They'd be affected probably

[josevalim]

Given it is a single function, can we make the contract a MFArgs instead of a module? This way we don't need to create new modules for every new adapter. We can just point to the existing function. Thank you.

[warmwaffles]

I don't know how I missed the SQLite adapter. They'd be affected probably

Don't worry I'm watching this 😈