WIP: Use default :enum value from database column's one

[ojab]

Reverts e991c7b and adds the test
Fixes #23187

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @sgrif (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[spastorino]

This is not the right fix.

Read e991c7b

I will check this out.

[ojab]

hmm, basically sqlite (in this example) returns string zero as string for 'PRAGMA table_info(books) with type integer and we're overwriting resulting ActiveModel::Type::Integer with ActiveRecord::Enum::EnumType in load_schema!.

So basically we should add type_cast somewhere, but I'm not sure where exactly where.
Actually sqlite3 (and at least postgres, I suggest, because this is reproducible on postgres) returns all integer integers as string (for example {"cid"=>1, "name"=>"author_id", "type"=>"integer", "notnull"=>0, "dflt_value"=>"0", "pk"=>0, 0=>1, 1=>"author_id", 2=>"integer", 3=>0, 4=>"0", 5=>0}), but .attributes_before_type_cast returns integers:

(byebug) Book.create.attributes_before_type_cast
{"id"=>1, "author_id"=>0, "format"=>nil, "name"=>nil, "status"=>nil, "read_status"=>nil, "nullable_status"=>nil, "language"=>nil, "author_visibility"=>nil, "illustrator_visibility"=>nil, "font_size"=>nil}

and type_cast actually happens in Attribute#value.

So the questions are:

• what exactly .attributes_before_type_cast should return?
• why type_cast happens when value is retrieval, not on value set?

[ojab]

So I think that basically type_cast should be moved to define_default_attribute or somewhere like that. I've pushed rails with this fix, please see the updated PR (count it as RFC, no commit message changes/etc was made). There are three test failures:

  1) Failure:
DefaultNumbersTest#test_default_positive_integer [/Users/ojab/Documents/repos/rails/activerecord/test/cases/defaults_test.rb:43]:
Expected: "7"
  Actual: 7


  2) Failure:
DefaultNumbersTest#test_default_decimal_number [/Users/ojab/Documents/repos/rails/activerecord/test/cases/defaults_test.rb:55]:
--- expected
+++ actual
@@ -1 +1 @@
-"2.78"
+#<BigDecimal:7fbe93366d58,'0.278E1',18(27)>



  3) Failure:
DefaultNumbersTest#test_default_negative_integer [/Users/ojab/Documents/repos/rails/activerecord/test/cases/defaults_test.rb:49]:
Expected: "-5"
  Actual: -5

but I'm not sure if this tests are correct.

If this fix looks proper -- I'll try to investigate if there is any more suitable place for type_cast and remove excessive type_casts further.

[ojab]

Aha, these tests are for before_type_cast values.
AFAICS there is inconsistency here and I don't know how rails should behave

(byebug) books(:tlg).dup.attributes_before_type_cast
{"id"=>nil, "author_id"=>1, "format"=>nil, "name"=>"Thoughtleadering", "status"=>"proposed", "read_status"=>"unread", "nullable_status"=>nil, "language"=>"english", "author_visibility"=>"visible", "illustrator_visibility"=>"visible", "font_size"=>"small"}
(byebug) books(:tlg).attributes_before_type_cast
{"id"=>4, "author_id"=>1, "format"=>nil, "name"=>"Thoughtleadering", "status"=>0, "read_status"=>0, "nullable_status"=>nil, "language"=>0, "author_visibility"=>0, "illustrator_visibility"=>0, "font_size"=>0}

Please advise, I'm lost.

[sgrif]

Attributes should never be cast on construction.