Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Grow from Small Simple Steps

Grow from Small Simple Steps

703496dd6f9097efaa51f2b8c83b47ac?s=128

aquajach

June 26, 2016
Tweet

Transcript

  1. Grow from Small Simple Steps Jack Chen @aquajach

  2. Grow from S3 Jack Chen @aquajach

  3. THE PROBLEM gem 'activerecord-import' books = [] 10.times do |i|

    books << Book.new(:name => "book #{i}") end Book.import books
  4. THE PROBLEM class Book < ActiveRecord::Base enum status: [:drafted, :finished,

    :published] #book.status = 0 or book.status = 'drafted' end create_table 'books' do |t| t.integer 'status', default: 0 end books = [] 10.times do |i| books << Book.new(:name => "book #{i}") end Book.import books #=> ArgumentError: '0' is not a valid status
  5. WHY IT FAILS # Returns the list of a table's

    column names, data types, and default values. def column_definitions(table_name) # :nodoc: exec_query(<<-end_sql, 'SCHEMA').rows SELECT a.attname, format_type(a.atttypid, a.atttypmod), pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass AND a.attnum > 0 AND NOT a.attisdropped ORDER BY a.attnum end_sql end
  6. WHY IT FAILS # Returns the list of a table's

    column names, data types, and default values. def column_definitions(table_name) # :nodoc: exec_query(<<-end_sql, 'SCHEMA').rows SELECT a.attname, format_type(a.atttypid, a.atttypmod), pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass AND a.attnum > 0 AND NOT a.attisdropped ORDER BY a.attnum end_sql end text
  7. WHY IT FAILS class ActiveRecord::Base class << self def import

    ... array_of_attributes = models.map do |model| column_names.map do |name| model.read_attribute_before_type_cast(name.to_s) end end ... instance = new do |model| hash.each_pair{ |k, v| model.send("#{k}=", v) } #=> ArgumentError: '0' is not a valid status end ... end end end
  8. TEST? class Book < ActiveRecord::Base enum status: [:draft, :published] if

    ENV['AR_VERSION'].to_i >= 4.1 #AR_VERSION = 3.1, 3.2, 4.0, 4.1, 4.2 end
  9. TEST? class Book < ActiveRecord::Base enum status: [:draft, :published] if

    ENV['AR_VERSION'].to_i >= 4.1 #AR_VERSION = 3.1, 3.2, 4.0, 4.1, 4.2 end
  10. None
  11. None
  12. It’s the best PR I have ever seen

  13. THE REALITY Only for PostgreSQL? Is Rails’ enum not smart

    enough?
  14. CRACK ON PR: read_attribute_before_type_cast => read_attribute Maintainer: why read_attribute_before_type_cast is

    used?
  15. Rails 1.2 Maintainer: It was back to 2007. Use read_attribute

    if it gets test case pass Maintainer: why read_attribute_before_type_cast is used?
  16. Maintainer: Another method handles casting. Using read_attribute might be ok

    Maintainer: It was back to 2007. Use read_attribute if it gets test case pass
  17. Me: Just benchmark it, read_attribute is 50% slower for casting

    Maintainer: Another method handles casting. Using read_attribute might be ok
  18. Committer: Simple fix instance = new do |model| # hash.each_pair{

    |k, v| model.send("#{k}=", v) } hash.each_pair{ |k,v| model[k] = v } end Me: Just benchmark it, read_attribute is 50% slower for casting
  19. Committer: Simple fix instance = new do |model| # hash.each_pair{

    |k, v| model.send("#{k}=", v) } hash.each_pair{ |k,v| model[k] = v } end ☹ Me: Seems working, but…
  20. Maintainer: I’m going to merge this and create another PR

    Me: Seems working, but…
  21. MY TAKEAWAYS • ActiveRecord::ConnectionAdapters • ActiveRecord different assignment methods and

    type casting • Default value in PostgreSQL • Libraries/gems supporting multiple ruby version and database adaptors • Rails enum
  22. Don’t believe in Stars

  23. Don’t think of making a perfect PR at the beginning

  24. None
  25. Don’t compromise