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 defaults are displayed as String unexpectedly #762

Closed
pocke opened this issue Feb 25, 2020 · 11 comments
Closed

Column defaults are displayed as String unexpectedly #762

pocke opened this issue Feb 25, 2020 · 11 comments

Comments

@pocke
Copy link

pocke commented Feb 25, 2020

Since v3.1.0, we got incorrect default values by annotate_models.

It is a part of our diff between v3.0.3 and v3.1.0.

-#  comments_count             :integer          default(0), not null
-#  comment_users_unique_count :integer          default(0), not null
-#  coediting                  :boolean          default(FALSE), not null
+#  comments_count             :integer          default("0"), not null
+#  comment_users_unique_count :integer          default("0"), not null
+#  coediting                  :boolean          default("false"), not null

And a part of schema.rb for the columns definitions.

    t.integer "comments_count", default: 0, null: false
    t.integer "comment_users_unique_count", default: 0, null: false
    t.boolean "coediting", default: false, null: false

I guess it depends on RDBMS because the spec is not wrong.
We use PostgreSQL.

Probably this pull request is related. #677

Commands

$ bin/rake annotate_models

And I investigated the ActiveRecord::ConnectionAdapters::PostgreSQLColumn#default returned value, which is used by #677, and klass.column_defaults[column.name], which was used previously.

$ bin/rails console

> klass.columns.find{|x| x.name == 'comments_count'}.default
=> "0"
> klass.column_defaults["comments_count"]
=> 0

Version

  • annotate version: v3.1.0
  • rails version: 5.2.4.1
  • ruby version: 2.6.4
@drwl
Copy link
Collaborator

drwl commented Feb 25, 2020

@pocke interesting, thank you for a detailed report and investigation 👍.

Does look like it also happens for sqlite per integration specs.

@drwl
Copy link
Collaborator

drwl commented Feb 25, 2020

@pocke if you have an instance of postgresql could you share what output you get from running these commands?

klass = SomeModelName # Replace this
pp Hash[klass.columns.map.with_index { |m, i| [i, m] }]
klass.column_defaults
irb(main):021:0> pp Hash[klass.columns.map.with_index { |m, i| [i, m] }]
{0=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9f90
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @name="id",
   @null=false,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972f0f20
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="integer",
     @type=:integer>,
   @table_name="tasks">,
 1=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9888
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @name="content",
   @null=true,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e98d8
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="varchar",
     @type=:string>,
   @table_name="tasks">,
 2=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9748
   @collation=nil,
   @comment=nil,
   @default="0",
   @default_function=nil,
   @name="count",
   @null=true,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e9798
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="integer",
     @type=:integer>,
   @table_name="tasks">,
 3=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9090
   @collation=nil,
   @comment=nil,
   @default="0",
   @default_function=nil,
   @name="status",
   @null=true,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e90e0
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="boolean",
     @type=:boolean>,
   @table_name="tasks">,
 4=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e89d8
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @name="created_at",
   @null=false,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e8a28
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="datetime",
     @type=:datetime>,
   @table_name="tasks">,
 5=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e8898
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @name="updated_at",
   @null=false,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e88e8
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="datetime",
     @type=:datetime>,
   @table_name="tasks">,
 6=>
  #<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e8758
   @collation=nil,
   @comment=nil,
   @default="0",
   @default_function=nil,
   @name="type_field",
   @null=true,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e87a8
     @limit=nil,
     @precision=nil,
     @scale=nil,
     @sql_type="integer",
     @type=:integer>,
   @table_name="tasks">}
=> {0=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9f90 @name="id", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972f0f20 @sql_type="integer", @type=:integer, @limit=nil, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, 1=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9888 @name="content", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e98d8 @sql_type="varchar", @type=:string, @limit=nil, @precision=nil, @scale=nil>, @null=true, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, 2=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9748 @name="count", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e9798 @sql_type="integer", @type=:integer, @limit=nil, @precision=nil, @scale=nil>, @null=true, @default="0", @default_function=nil, @collation=nil, @comment=nil>, 3=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e9090 @name="status", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e90e0 @sql_type="boolean", @type=:boolean, @limit=nil, @precision=nil, @scale=nil>, @null=true, @default="0", @default_function=nil, @collation=nil, @comment=nil>, 4=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e89d8 @name="created_at", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e8a28 @sql_type="datetime", @type=:datetime, @limit=nil, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, 5=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e8898 @name="updated_at", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e88e8 @sql_type="datetime", @type=:datetime, @limit=nil, @precision=nil, @scale=nil>, @null=false, @default=nil, @default_function=nil, @collation=nil, @comment=nil>, 6=>#<ActiveRecord::ConnectionAdapters::Column:0x00007f99972e8758 @name="type_field", @table_name="tasks", @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007f99972e87a8 @sql_type="integer", @type=:integer, @limit=nil, @precision=nil, @scale=nil>, @null=true, @default="0", @default_function=nil, @collation=nil, @comment=nil>}
irb(main):022:0>
irb(main):023:0> klass.column_defaults
=> {"id"=>nil, "content"=>nil, "count"=>0, "status"=>false, "created_at"=>nil, "updated_at"=>nil, "type_field"=>"inactive"}

@drwl
Copy link
Collaborator

drwl commented Feb 25, 2020

My example:

# Task.rb
class Task < ApplicationRecord
  enum type_field: { inactive: 0, active: 1, archived: 2 }
end

# schema.rb
ActiveRecord::Schema.define(version: 2020_02_25_152254) do

  create_table "tasks", force: :cascade do |t|
    t.string "content"
    t.integer "count", default: 0
    t.boolean "status", default: false
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.integer "type_field", default: 0
  end

end

@pocke
Copy link
Author

pocke commented Feb 25, 2020

Thanks for your investigation!

@pocke if you have an instance of postgresql could you share what output you get from running these commands?

Sure! The following is the output with the same tasks table.

pp Hash[klass.columns.map.with_index { |m, i| [i, m] }]
=> {0=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x00005643177790b0
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function="nextval('tasks_id_seq'::regclass)",
   @max_identifier_length=63,
   @name="id",
   @null=false,
   @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00005643177795d8 @limit=8, @precision=nil, @scale=nil, @sql_type="bigint", @type=:integer>,
   @table_name="tasks">,
 1=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564317778a70
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @max_identifier_length=63,
   @name="content",
   @null=true,
   @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317778b88 @limit=nil, @precision=nil, @scale=nil, @sql_type="character varying", @type=:string>,
   @table_name="tasks">,
 2=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564317778480
   @collation=nil,
   @comment=nil,
   @default="0",
   @default_function=nil,
   @max_identifier_length=63,
   @name="count",
   @null=true,
   @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317778610 @limit=4, @precision=nil, @scale=nil, @sql_type="integer", @type=:integer>,
   @table_name="tasks">,
 3=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564317783ec0
   @collation=nil,
   @comment=nil,
   @default="false",
   @default_function=nil,
   @max_identifier_length=63,
   @name="status",
   @null=true,
   @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317778020 @limit=nil, @precision=nil, @scale=nil, @sql_type="boolean", @type=:boolean>,
   @table_name="tasks">,
 4=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564317783880
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @max_identifier_length=63,
   @name="created_at",
   @null=false,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317783998 @limit=nil, @precision=nil, @scale=nil, @sql_type="timestamp without time zone", @type=:datetime>,
   @table_name="tasks">,
 5=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x0000564317783560
   @collation=nil,
   @comment=nil,
   @default=nil,
   @default_function=nil,
   @max_identifier_length=63,
   @name="updated_at",
   @null=false,
   @sql_type_metadata=
    #<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317783678 @limit=nil, @precision=nil, @scale=nil, @sql_type="timestamp without time zone", @type=:datetime>,
   @table_name="tasks">,
 6=>
  #<ActiveRecord::ConnectionAdapters::PostgreSQLColumn:0x00005643177831c8
   @collation=nil,
   @comment=nil,
   @default="0",
   @default_function=nil,
   @max_identifier_length=63,
   @name="type_field",
   @null=true,
   @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x0000564317783358 @limit=4, @precision=nil, @scale=nil, @sql_type="integer", @type=:integer>,
   @table_name="tasks">}


klass.column_defaults
{"id"=>nil, "content"=>nil, "count"=>0, "status"=>false, "created_at"=>nil, "updated_at"=>nil, "type_field"=>"inactive"}

@gingerlime
Copy link

Also seeing it, and we're also using PG. Let me know if there's anything I can help/test :)

@drwl
Copy link
Collaborator

drwl commented Feb 29, 2020

Just gonna revert #677 for now and make a feature request for doing enums.

@drwl
Copy link
Collaborator

drwl commented Feb 29, 2020

@gingerlime @pocke can yall test this branch? #768

davidtaylorhq added a commit to discourse/discourse that referenced this issue Mar 2, 2020
v3.1.0 has a bug which rewrites default annotations with erroneous quotes. ctran/annotate_models#762

This reverts commit dd4a04e.
drwl added a commit that referenced this issue Mar 8, 2020
It was reported in #762 that column defaults were broken. This reverts changes made in #677 to restore the expected behavior of column defaults. 

For the time being columns with associated enums won't be working.
@drwl
Copy link
Collaborator

drwl commented Mar 24, 2020

This should be fixed in v3.1.1 which just got released.

@pocke
Copy link
Author

pocke commented Mar 24, 2020

Thanks for releasing it! I've confirmed the problem has been fixed in v3.1.1.

@gingerlime
Copy link

Same here! Thank you 👍

@drwl
Copy link
Collaborator

drwl commented Mar 24, 2020

Great to hear -- I apologize for the delay, life has been a bit craazy recently. Going to close this out. Feel free to reopen.

@drwl drwl closed this as completed Mar 24, 2020
vfonic pushed a commit to vfonic/annotate_models that referenced this issue May 8, 2020
It was reported in ctran#762 that column defaults were broken. This reverts changes made in ctran#677 to restore the expected behavior of column defaults. 

For the time being columns with associated enums won't be working.
dazralsky pushed a commit to dazralsky/annotate_models that referenced this issue Aug 21, 2023
It was reported in ctran/annotate_models#762 that column defaults were broken. This reverts changes made in #677 to restore the expected behavior of column defaults. 

For the time being columns with associated enums won't be working.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants