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
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
26. Don’t compromise