constraint_validations with class_table_inheritance

[sterlzbd]

I'm running into an issue using class_table_inheritance with the constraint_validations plugin. When I'm expecting to get a Sequel::ValidationFailed error I'm instead getting Sequel::CheckConstraintViolation errors

Here is a small example

DB.extension(:constraint_validations)
DB.create_constraint_validations_table

DB.create_table :employees do
  primary_key :id
  column :kind, :text, null: false
  constraint :kinds, kind: %w( Manager )

  column :name, :text
  column :status, :text

  validate do
    includes %w( active leave terminated ), :status, allow_nil: false, name: :statuses
  end
end

DB.create_table :managers do
  foreign_key :id, :employees, null: false
  primary_key [:id]

  column :level, :text

  validate do
    includes %w( junior senior executive ), :level, allow_nil: true, name: :levels
  end
end

class Employee < Sequel::Model
  plugin :class_table_inheritance, key: :kind
  plugin :constraint_validations
end

class Manager < Employee
end

begin
  # violate Employee statuses check constraint
  Manager.create name: "Bob", status: "nonsense"
rescue Sequel::ValidationFailed
  # Hoping for a Sequel::ValidationFailed error
  puts "Sequel::ValidationFailed"
rescue Sequel::CheckConstraintViolation
  # but getting a Sequel::CheckConstraintViolation error
  puts "Sequel::CheckConstraintViolation"
end

begin
  # violates Manager levels check constraint
  Manager.create name: "Bob", status: "active", level: "nonsense"
rescue Sequel::ValidationFailed
  # Hoping for a Sequel::ValidationFailed error
  puts "Sequel::ValidationFailed"
rescue Sequel::CheckConstraintViolation
  # but getting a Sequel::CheckConstraintViolation error
  puts "Sequel::CheckConstraintViolation"
end

In both of those I was hoping to trigger a Sequel::ValidationFailed but both cases are raising a Sequel::CheckConstraintViolation

I think it's related to the table name that the plugin is using to look up the constraint in the validations table. Using the code from parse_constraint_validations

sequel/lib/sequel/plugins/constraint_validations.rb

Lines 125 to 126 in e0c99f6

	ds = @dataset.with_quote_identifiers(false)
	table_name = ds.literal(ds.first_source_table)

The Manager table_name is

(SELECT "employees"."id", "employees"."kind", "employees"."name", "employees"."status", "managers"."level" FROM "employees" INNER JOIN "managers" ON ("managers"."id" = "employees"."id"))

The tables in the constraint validations table look correct to me

cit_auto_validations_test=> select * from sequel_constraint_validations;
   table   | constraint_name |  validation_type   | column |        argument         | message | allow_nil
-----------+-----------------+--------------------+--------+-------------------------+---------+-----------
 employees | statuses        | includes_str_array | status | active,leave,terminated |         | f
 managers  | levels          | includes_str_array | level  | junior,senior,executive |         | t

[jeremyevans]

I don't want to make constraint_validations or class_table_inheritance to have dependencies on the other one. The best fix in this case would be a small plugin that depends on both and has the necessary glue code. Does that sound like something you would be willing to work on? If not, I can add this to my todo list, but it wouldn't be high priority.

Also, not sure if you are using PostgreSQL, but if so, you could also consider the pg_auto_constraint_validations plugin. It works a little differently, rescuing the Sequel::CheckConstraintViolation and reraising as a ValidationFailed.

[sterlzbd]

Yeah that makes sense. We're already pretty dependent on constraint_validations but we are using Posrgres so maybe pg_auto_constraint_validations would be a better fit. I'll check that out!

I wouldn't mind adding a little plugin to glue the two plugins together either. Do you have any inkling of how a plugin like that would work?

Maybe the models in the CTI hierarchy would override their @constraint_validations or maybe we could extend the constraint_validations to take an option with table_name(s) overrides to construct @constraint_validations from.

[jeremyevans]

I would add a parse_constraint_validations_dataset method to constraint_validations, and move the if @dataset logic from parse_constraint_validations to that method. Then have the new plugin override that method to handle all CTI tables instead of just the first one.

[sterlzbd]

I finally got around to adding a small plugin for this but i'm not convinced I did it the best way. I ended up using the cti_tables variable from class_table_inheritance in the suggested parse_constraint_validations_dataset method. The problem with that is constraint_validations runs parse_constraint_validations in the after_set_dataset hook but class_table_inheritance updates cti_tables after it sets the dataset. To get around that I had my little plugin rerun parse_constraint_validations at the end of inherited so it would have the updated cti_tables variable to use. That works but it doesn't feel very sequel-ish to run that method twice for each model in the CTI hierarchy.

Anyways, here's my branch if you want to take a look: master...sterlzbd:sequel:cti_and_constraint_validations

If you think that's on the right track I can add a PR.

[jeremyevans]

Thank you for working on this. I agree that having to call the method twice isn't best, but there doesn't seem to be an easy way around it. With the return unless @dataset at the top of the method, it shouldn't be a problem, as it doesn't run the logic twice.

Please submit a PR for this. Can you rename the plugin to class_table_inheritance_constraint_validations? Also, if possible, can you add integration tests for this (integration tests test with a real database)? spec/integration/plugin_test.rb already has tests for both class_table_inheritance and constraint_validations, so you should be able to copy from those tests.

[sterlzbd]

No problem! I'll add integration tests and then open a PR.