Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix erroneous nil default precision on virtual datetime columns #46110

Merged
merged 2 commits into from Sep 22, 2022
Merged

Fix erroneous nil default precision on virtual datetime columns #46110

merged 2 commits into from Sep 22, 2022

Commits on Sep 22, 2022

  1. Add test for existing virtual datetime precision 

    Active Record erroneously uses a precision of `nil` instead of the default
precision, when adding a virtual datetime column.

This tests that behavior ahead of fixing it.
    @sambostock
    sambostock committed Sep 22, 2022
    Configuration menu
    Copy the full SHA
    919dd4a View commit details
    Browse the repository at this point in the history

  2. Fix virtual datetime default precision 

    Prior to this change

    t.virtual :column_name, type: :datetime

would erroneously produce the same result as

    t.virtual :column_name, type: :datetime, precision: nil

This is because the code path for `virtual` skipped the default lookup:

- `t.datetime` is delegated to the `column` method
  - `column` sees `type == :datetime` and sets a default `precision` is none was given
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
- `t.virtual` is delegated to the `column` method
  - `column` sees `type == :virtual`, not `:datetime`, so skips the default lookup
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`

By moving the default lookup, we get consistent code paths:

- `t.datetime` is delegated to the `column` method
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given
- `t.virtual` is delegated to the `column` method
  - `column` calls `new_column_definition` to add the result to `@columns_hash`
    - `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`
    - `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given
    @sambostock
    sambostock committed Sep 22, 2022
    Configuration menu
    Copy the full SHA
    b85ee91 View commit details
    Browse the repository at this point in the history