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.

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

Column default incorrectly showing as NULL #935

Closed
bjeanes opened this issue Feb 15, 2022 · 3 comments
Closed

Column default incorrectly showing as NULL #935

bjeanes opened this issue Feb 15, 2022 · 3 comments

Comments

@bjeanes
Copy link

bjeanes commented Feb 15, 2022

I have two models, both with a state field, both with a default value. annotate_models correctly reflected the default for both, until I ran a migration with change_column :table, :col, :type, default: 'new-default'. Now, the changed default is documented as default(NULL), not null (which doesn't even make sense) and I can't get it to show the correct default anymore, even after tearing down DB and re-running all migrations.

This is a pretty brand new app, undeployed, not even any controllers yet... I have no theory for how/why this is happening.

Migration:

class ChangeIdentityStateDefault < ActiveRecord::Migration[7.0]
  def up
    change_column :identities, :state, :text, default: 'submitted'
    Identity.where(state: 'pending_review').update_all(state: 'submitted')
  end

  def down
    change_column :identities, :state, :text, default: 'pending_review'
    Identity.where(state: 'submitted').update_all(state: 'pending_review')
  end
end

psql of both tables:

beekeeper=# \d identities
                   Table "public.identities"
    Column    | Type | Collation | Nullable |      Default
--------------+------+-----------+----------+-------------------
 id           | uuid |           | not null | gen_random_uuid()
 callback_url | text |           | not null |
 state        | text |           | not null | 'submitted'::text
Indexes:
    "identities_pkey" PRIMARY KEY, btree (id)
Referenced by:
    TABLE "identifications" CONSTRAINT "fk_rails_403e5f005a" FOREIGN KEY (identity_id) REFERENCES identities(id)

beekeeper=# \d identifications
                              Table "public.identifications"
      Column      |            Type             | Collation | Nullable |      Default
------------------+-----------------------------+-----------+----------+-------------------
 id               | uuid                        |           | not null | gen_random_uuid()
 identity_id      | uuid                        |           | not null |
 state            | text                        |           | not null | 'submitted'::text
 checks           | jsonb                       |           |          |
 claimed_age      | text                        |           | not null |
 submitter_notes  | text                        |           |          |
 submitted_at     | timestamp without time zone |           |          |
 created_at       | timestamp(6) with time zone |           | not null |
 updated_at       | timestamp(6) with time zone |           | not null |
 expires_at       | timestamp(6) with time zone |           |          |
 rejection_reason | text                        |           |          |
Indexes:
    "identifications_pkey" PRIMARY KEY, btree (id)
    "index_identifications_on_identity_id" btree (identity_id)
Foreign-key constraints:
    "fk_rails_403e5f005a" FOREIGN KEY (identity_id) REFERENCES identities(id)
Referenced by:
    TABLE "identification_state_transitions" CONSTRAINT "fk_rails_7c40242644" FOREIGN KEY (identification_id) REFERENCES identifications(id)

The comments generated since running the migration:

# == Schema Information
#
# Table name: identities
#
#  id           :uuid             not null, primary key
#  callback_url :text             not null
#  state        :text             default(NULL), not null
#
class Identity < ApplicationRecord
# == Schema Information
#
# Table name: identifications
#
#  id               :uuid             not null, primary key
#  checks           :jsonb
#  claimed_age      :text             not null
#  expires_at       :datetime
#  rejection_reason :text
#  state            :text             default("submitted"), not null
#  submitted_at     :timestamp
#  submitter_notes  :text
#  created_at       :datetime         not null
#  updated_at       :datetime         not null
#  identity_id      :uuid             not null, indexed
#
# Indexes
#
#  index_identifications_on_identity_id  (identity_id)
#
# Foreign Keys
#
#  fk_rails_...  (identity_id => identities.id)
#
class Identification < ApplicationRecord

Commands

The default is incorrect (and overwrites manual corrections) both when running a post-migration rake hook and when running bundle exec annotate directly.

Version

  • ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [aarch64-linux-musl]
  • Rails 7.0.0
  • annotate 3.2.0
@bjeanes
Copy link
Author

bjeanes commented Feb 15, 2022

Strangely, if I run rails db:rollback the comment shows the restored default string DEFAULT("pending_review"):

bash-5.1# rails db:rollback
== 20220215062631 ChangeIdentityStateDefault: reverting =======================
-- change_column(:identities, :state, :text, {:default=>"pending_review"})
   -> 0.0041s
== 20220215062631 ChangeIdentityStateDefault: reverted (0.0562s) ==============

Annotated (1): app/models/identity.rb
# == Schema Information
#
# Table name: identities
#
#  id           :uuid             not null, primary key
#  callback_url :text             not null
#  state        :text             default("pending_review"), not null
#
class Identity < ApplicationRecord

Further, if I edit the migration to have any default string other than 'pending_review, annotate documents the default as NULL and if I change even the up part to re-set the default as pending_review it documents it correctly.

This makes no sense to me...

@bjeanes
Copy link
Author

bjeanes commented Feb 15, 2022

Oh damn... I think it's related to #768 or #677 or something in those code paths, but that at least allowed me to fix it.

The issue was that an enum on the model hadn't yet been changed to include the new value. I'm assuming there is some code in annotate to map the enum values to/from the raw DB value and this was returning a nil in Ruby, which was mis-described as a NULL from DB.

@bjeanes
Copy link
Author

bjeanes commented Feb 15, 2022

Related: #839

This is VERY surprising behaviour. I expected this to be documenting the database schema and its defaults, not mapping to/from application-level values (which in my case are the same as my enum is enum :state, {submitted: 'submitted', foo: 'foo', bar: 'bar', ...}).

@bjeanes bjeanes closed this as completed Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant