Possible bug with min/max auto validations in 5.62 update

[awei82]

Hi @jeremyevans, I'm updating my Sequel version from 5.61 to 5.67, and I ran into this error when creating a new model object:

/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:151:in `>': comparison of String with 2147483647 failed (ArgumentError)

            validation_error_message(m, max) if !v.nil? && v > max
                                                               ^^^
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:151:in `block in validates_max_value'
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:323:in `block in validatable_attributes'
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:317:in `each'
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:317:in `validatable_attributes'
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:332:in `validatable_attributes_for_type'
.../sequel-5.67.0/lib/sequel/plugins/validation_helpers.rb:150:in `validates_max_value'
.../sequel-5.67.0/lib/sequel/plugins/auto_validations.rb:280:in `block in validate'
.../sequel-5.67.0/lib/sequel/plugins/auto_validations.rb:279:in `each'
.../sequel-5.67.0/lib/sequel/plugins/auto_validations.rb:279:in `validate'

This looks like it's being caused by the min/max validations helpers added in 5.62.0.

After digging into the error, I realized that my table has an enum column, and the auto-validator is confusing it for an integer column. It looks like the validation checking code picks up enum columns by accident?

This is what my enum column looks like in the db_schema object:

:type=>
  {:oid=>36042,
   :db_type=>"intro_type",
   :default=>nil,
   :allow_null=>false,
   :primary_key=>false,
   :generated=>false,
   :type=>:enum,
   :ruby_default=>nil,
   :min_value=>-2147483648,
   :max_value=>2147483647,
   :enum_values=>["request", "offer"]},

And the code used to identify min/max value columns scans for the :min_value and :max_value fields, but doesn't filter out enum types: https://github.com/jeremyevans/sequel/blob/master/lib/sequel/plugins/auto_validations.rb#L226

In the meantime, I've suppressed the error by adding skip_auto_validations :max_value and skip_auto_validations :min_value to the model.

Just wanted to share this to confirm whether or not this is a legitimate bug. Thanks!

[jeremyevans]

Thanks for the report. This is a bug. We shouldn't set a min_value/max_value for enum columns. The reason you have this issue is your column name starts with int, so it gets treated as an integer type, which is why the min_value/max_value is set. We can probably work around this by having the pg_enum extension delete min_value/max_value for enums. I'll work on that tomorrow.

[jeremyevans]

Theoretically, we could do so for PostgreSQL and make the type parsing purely use OIDs . However, Sequel supports 10+ other database types, and most of those require parsing type names (and don't support custom types). I'm not sure it's worth changing this for PostgreSQL, considering it very rarely causes issues, but I guess I'm open to that if someone wants to work on it and can make it backwards compatible.

[AlexWayfer]

Sorry, @jeremyevans, just want to clarify.

your column name starts with int

Sequel supports 10+ other database types, and most of those require parsing type names

So, should Sequel parse "column names" or "type names"? Or probably you meant "parse type in column names"…

Sequel is an ORM for relational DBs, as I know. And I respect its DB support, and not an expert in all of 10+ DBs, but really most of them don't have an integer column type? Or ways to request column types?

Sorry for bothering, just discussion, to understand and deal with such edge-cases in future.

[jeremyevans]

So, should Sequel parse "column names" or "type names"? Or probably you meant "parse type in column names"…

No, I said parsing type names because I meant parsing type names. Sequel's type parsing is based on type names, because that is the common denominator for all databases Sequel supports.

Sequel is an ORM for relational DBs, as I know. And I respect its DB support, and not an expert in all of 10+ DBs, but really most of them don't have an integer column type? Or ways to request column types?

Most databases have integer column types (usually multiple, for different sized integers). Oracle is an exception. In terms of "ways to request column types", it's common for the database driver to provide this as a type name (which can vary across databases). Sequel needs to parse the type name to determine which Ruby type to map it to.

Sorry for bothering, just discussion, to understand and deal with such edge-cases in future.

You should probably review the schema_parse_table methods for all adapters, to see the many different ways that Sequel needs to get schema information and try to provide a unified API to different databases.

Looking at the PostgreSQL documentation, instead of switching to a OID based type mapping, we can probably use typtype and typarray in pg_type to skip the default type parsing for custom types, and always use :enum for enum types. I'll try to give that a shot tomorrow as well.

[jeremyevans]

@awei82 I pushed 96486c2 with the necessary changes. Can you please test the master branch in your environment and see if the issue is fixed?

[awei82]

I've tested it and it's working again. Thanks!