Grow from Small Simple Steps

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. ☹

22. 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

23. Don’t believe in Stars

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

25. None

26. Don’t compromise