What steps will reproduce the problem? 1. Set up source code and migration: https://gist.github.com/900535 2. In console, run "j = Job.create {|job| job.args = {}}" 3. Then run "j[:id] = 4" 4. Finally, check "j.modified?"

What is the expected output? What do you see instead? I expect to see true, but see false instead

What version of the product are you using? On what operating system? I'm running the edge version of Sequel (Gemfile.lock has d737ec).

Please provide any additional information below.

There might be technical reasons that this is "hard", i.e. args are serialized and deserialized lazily, but this means my_job.save_changes doesn't work if the modified fields are serialized! Not exactly intuitive, since from the developer's perspective whether a field is in fact serialized or not should generally be inconsequential.

Can't replicate:

$ ruby -I lib bin/sequel -E sqlite:/ I, [2011-04-03T11:50:10.932317 #19273] INFO -- : (0.000137s) PRAGMA foreign_keys = 1 Your database is stored in DB... irb(main):001:0> DB.create_table :jobs do irb(main):002:1* primary_key :id, :type => Bignum irb(main):003:1> String :args, :text => true irb(main):004:1> end I, [2011-04-03T11:50:11.507844 #19273] INFO -- : (0.015221s) CREATE TABLE jobs (id integer PRIMARY KEY AUTOINCREMENT, args text) => nil irb(main):005:0> class Job < Sequel::Model irb(main):006:1> plugin :serialization, :marshal, :args irb(main):007:1> end I, [2011-04-03T11:50:11.567788 #19273] INFO -- : (0.000390s) PRAGMA table_info('jobs') => [:args] irb(main):008:0> j = Job.create {|job| job.args = {}} I, [2011-04-03T11:50:11.627654 #19273] INFO -- : (0.000254s) SELECT sqlite_version() LIMIT 1 I, [2011-04-03T11:50:11.627908 #19273] INFO -- : (0.000070s) BEGIN I, [2011-04-03T11:50:11.641989 #19273] INFO -- : (0.007349s) INSERT INTO jobs (args) VALUES ('BAh7AA== ') I, [2011-04-03T11:50:11.657095 #19273] INFO -- : (0.007318s) SELECT * FROM jobs WHERE (id = 1) LIMIT 1 I, [2011-04-03T11:50:11.664733 #19273] INFO -- : (0.000156s) COMMIT => #"BAh7AA==\n", :id=>1}> irb(main):009:0> j.modified? => false irb(main):010:0> j[:id] = 4 => 4 irb(main):011:0> j.modified? => true

I can see you getting false for modified if you provided the same value as the existing value of :id.

Now, I didn't just run your own code, I figured out you missed something important (j.args[:id] instead of j[:id]), which you are correct, doesn't work that way. I thought about it for a while, and I think the patch below will fix it, at the expense of repeated serialization of objects. Also, it could have false positives, since identical objects can serialize differently (e.g. hashes depending on order). I'm wary to apply the patch because of the false positives and because of the performance hit it will cause for people serializing large objects.

I'm currently labeling this a WontFix. You can post on the sequel-talk Google Group if you want this changed by default, and depending on the response there, I may decide to apply it. If it doesn't get applied by default (or until it does), just add the new instance methods in this patch to the Job class:

diff --git a/lib/sequel/plugins/serialization.rb b/lib/sequel/plugins/serialization.rb index 8280bfc..000c8f8 100644 --- a/lib/sequel/plugins/serialization.rb +++ b/lib/sequel/plugins/serialization.rb @@ -121,6 +121,16 @@ module Sequel super end

• def changed_columns
• cc = super
• deserialized_values.each{|c, v| cc << c if !cc.include?(c) && serialize_value(c, v) != self[c]}
• cc
• end +
• def modified?
• super || deserialized_values.any?{|c, v| serialize_value(c, v) != self[c]}
• end + # Empty the deserialized values when refreshing. def refresh @deserialized_values = {}

I decided to add a new plugin that you can use to detect modifications to serialized columns: https://github.com/jeremyevans/sequel/commit/dd9a1ee36970b767a6b0816b4c11778d6881951e

Thanks Jeremy. You're right, I screwed up the repro -- I did mean j.args[:id], not j[:id]. I'd imagined two solutions -- one where modified? reserializes all the parameters (which you mentioned) and another where Sequel somehow uses AOP to intercept calls to all the setters, which...wouldn't work great.

Thanks for the solution -- I think opt-in is appropriate given the nature of the fix.