Better introspection integration with ActiveRecord::DB_DEFAULT?

[tovodeverett]

I experimented with default: :now today, and it appears that (at least in Rails 3.2 - I haven't tested Rails 4.0 yet) the developer must ensure that the defaulted attributes get ActiveRecord::DB_DEFAULT assigned to them, even during record creation.

I'm wondering if it's desirable (and feasible) to hook into the ActiveRecord code that sets attributes to their default values during new so that it supplies ActiveRecord::DB_DEFAULT for attributes for columns that have expression defaults.

On a parallel note, when Rails inserts a new record with an auto-generated ID column, it uses RETURNING in the SQL to get the ID column value and updates the record to reflect that. I wonder if it's desirable (and feasible) to also have it return the values for the columns that uses ActiveRecord::DB_DEFAULT. In the absence of that I used reload, but it would reduce database round trips and improve the developer experience if it were handled automatically.

Thoughts?

[ronen]

I'm wondering if it's desirable (and feasible) to hook into the ActiveRecord code that sets attributes to their default values during new so that it supplies ActiveRecord::DB_DEFAULT for attributes for columns that have expression defaults.

Hmm. Good question. Summarizing your proposed change:

	Argument	Current db value	Proposed db value
(a)	column: nil	NULL	NULL
(b)	not provided	NULL	default
(c)	column: ActiveRecord::DB_DEFAULT	default	default

From a 'pure' sense this makes sense to me and I like it: if you don't specify anything at all you get the default; and if you want NULL then you specify column: nil. You shouldn't have to specify "I WANT THE DEFAULT" (in all caps") to get a default value.

My hesitations are:

• Will it confuse people? In most libraries I'd guess leaving off a hash argument is the same as providing nil. So the proposed behavior, sensible though it may be, might be non-standard enough to cause some confusion.
• Will it break existing code? Possibly, but I guess that's what major version numbers are for.
• Is it possible? If ActiveRecord fleshes out the argument hash with nil values before we can hook into it, we'd have trouble detecting circumstances where NULL is really what's wanted.

Regarding that last point, we could get even more outlandish, but still sensible in its own way:

	Argument	Current db value	Proposed db value
(a)	column: nil	NULL	default
(b)	not provided	NULL	default
(c)	column: ActiveRecord::DB_DEFAULT	default	default
(c)	column: ActiveRecord::DB_NULL	n/a	NULL

For columns that don't have a database default defined, the "default" value would be to insert NULL, same as always. But for columns that do have a a default database defined, if you want to override the default and insert a NULL you'd need to go out of your way to specify DB_NULL.

On a parallel note, when Rails inserts a new record with an auto-generated ID column, it uses RETURNING in the SQL to get the ID column value and updates the record to reflect that. I wonder if it's desirable (and feasible) to also have it return the values for the columns that uses ActiveRecord::DB_DEFAULT. In the absence of that I used reload, but it would reduce database round trips and improve the developer experience if it were handled automatically.

Yeah, good idea. Regardless of what argument mechanism we use to tell rails to use the database default. After saving a model instance you'd expect the instance and the database record to be in sync. So that means if the database computed some column value it'd makes sense to propagate it back to the instance.

[tovodeverett]

My understanding of the Rails 3.2 behavior is that it supplies the default values when you instantiate a new model instance. For instance, run the following migration (using schema_plus):

  def change
    create_table :demos do |t|
      t.integer :demo_1, default: 42
      t.datetime :demo_2, default: '2001-09-11'
      t.datetime :demo_3, default: :now

      t.timestamps
    end
  end

Then create a new record:

>> Demo.new
=> #<Demo id: nil, demo_1: 42, demo_2: "2001-09-11 00:00:00", demo_3: nil, created_at: nil, updated_at: nil>

Notice how it automatically supplies the default values for the columns with non-expression defaults. Furthermore, if you execute Demo.columns, you'll see how the first two columns have values for @default, but the :demo_3 column has @default=nil, @default_expr="now()",. I tried using instance_exec to modify @default and had issues because Singleton doesn't like dup, but as soon as I defined a simple SchemaPlus::ActiveRecord::DbDefault#dup that just returned self, then Demo.new did this:

>> Demo.new
=> #<Demo id: nil, demo_1: 42, demo_2: "2001-09-11 00:00:00", demo_3: #<SchemaPlus::ActiveRecord::DbDefault:0x00000002bbf740>, created_at: nil, updated_at: nil>

My instinct is that this would be the desired behavior - we basically mimic as closely as possible what Rails already does for default values. We can't actually supply the default value because it needs to be calculated by the database when the record is saved, but we can do the next best thing. Then if someone explicitly assigns nil, it's just the same as if they explicitly assigned nil to a column with a non-expression default - they get NULL. This also means there's no need to worry about ActiveRecord::DB_NULL. My recommendation would be to leave non-expression default columns alone - there's no need to modify the existing Rails behavior.

I suspect that fixing PostgreSQL (if I'm reading the code correctly) might be as simple as modifying SchemaPlus::ActiveRecord::ConnectionAdapters::PostgreSQLColumn#initialize like so:

        def initialize(name, default, sql_type = nil, null = true)
          if default.is_a? Hash
            if default[:expr]
              @default_expr = default[:expr]
              default = ActiveRecord::DB_DEFAULT
            else
              default = nil
            end
          end
          super(name, default, sql_type, null)
        end

I'll try to get to it this weekend or early this week. I'll create a branch for all of this so we can evaluate when we want to pull it in. I agree that it does require a major version change since it is technically a backwards incompatibility, even though I think this new behavior is closer to what people expect. As such, we might want to wait until after @donv's work gets merged in.

I'll also investigate approaches for keeping the model and the database in sync on a save.

[tovodeverett]

This is getting to be even more fun. Rails 4 appears to change the behavior of nils when it comes to inserting - if the @default for the column is nil and the value doesn't change, then it doesn't specify that column during the insert. This means that in Rails 4, ActiveRecord::DB_DEFAULT is technically not needed for a column with an expression default, but it does mean you can't get a NULL by specifying nil! The good news is the approach I outlined above does work, and appears to work equally well in both Rails 3 and Rails 4 for PostgreSQL because ActiveRecord::DB_DEFAULT isn't nil and so it puts both of them on the same playing field (and I think it's a reasonable one).

The bad news is that since SQLite3 doesn't support DEFAULT (at least according to https://github.com/lomba/schema_plus#column-defaults-using), that means that the observed behavior of columns with expression defaults in SQLite3 changes from Rails 3 to Rails 4 - in Rails 3 you can't take advantage of the default, and in Rails 4 you can't avoid the default! All of this is making the test suite fun to write!

[tovodeverett]

I just pushed 42f1ea8 to https://github.com/lomba/schema_plus/tree/default_introspection. It has the ActiveRecord::DB_DEFAULT support for columns with expression defaults for PostgreSQL. There's no way I see to support this in either MySQL or SQLite3 since MySQL doesn't support expression defaults and SQLite3 doesn't support the specifying DEFAULT for a column value.

I haven't attacked post-insert/update synchronization yet.

[ronen]

yowza. amazing fact-finding mission you went on!

i'm thinking the README should have a table or tables listing the behavior for:

• column value not provided vs. set to nil vs. set to DB_DEFAULT
• db specifies no default vs constant default vs default expression
• new vs update
• rails 3.2 vs 4
• postgresql vs sql vs sqlite
• with vs. without schema_plus? (with your updates of course)

not sure what the best way to format it would be... of course there'd be plenty of cells that would be N/A

[if we were very super-clever, that table would be built from a json or xml data file, which we would also use to drive the specs. 1/2 😄 ]