dkubb / axiom

Simplifies querying of structured data using relational algebra
https://github.com/dkubb/axiom
MIT License
459 stars 22 forks source link

Freeze behavior limits tuple Enumerator implementations #28

Closed ahamid closed 11 years ago

ahamid commented 11 years ago

I am upgrading from Veritas to Axiom and noticed that my custom tuple enumerator is breaking now because of automatic object graph freezing that is now happening. Example that illustrates the problem:

require 'adamantium'

# both must be true to avoid exception
DONT_AUTO_FREEZE=false
OVERRIDE_FREEZE_OBJECT=false

if DONT_AUTO_FREEZE
  module Adamantium
    module ClassMethods
      def new(*)
        #freezer.freeze(super)
        super
      end
    end
  end
end

require 'axiom'

if OVERRIDE_FREEZE_OBJECT
  Axiom::Relation.class_eval do
    private
    def freeze_object(object); object; end
  end
end

class CustomEnumerator < Enumerator
  def initialize(tuple_source)
    @array_will_freeze = []
    super() do |y|
      @array_will_freeze << "sadf"
      tuple_source.each do |t|
        p t
        y << t
      end
    end
  end
end

tuple_enumerator = CustomEnumerator.new([[1, "one"],[2,"two"],[3,"three"],[4,"four"],[5,"five"]])
relation = Axiom::Relation.new([ [ :id, Integer ], [ :value, String ] ], tuple_enumerator)

p relation.restrict { |r| r.id.gt(2).and(r.id.lt(4)) }.to_a
p relation.restrict { |r| r.id.gt(1).and(r.id.lt(3)) }.to_a
p relation.restrict { |r| r.id.gt(3).and(r.id.lt(5)) }.to_a

if both changes enabled by DONT_AUTO_FREEZE and OVERRIDE_FREEZE_OBJECT are not enabled, the following exception occurs:

axiom_test.rb:30:in `block in initialize': can't modify frozen Array (RuntimeError)
    from /home/aaron/.rvm/gems/ruby-1.9.3-p429@flatfiles/gems/axiom-0.1.0/lib/axiom/relation.rb:109:in `each'
    from /home/aaron/.rvm/gems/ruby-1.9.3-p429@flatfiles/gems/axiom-0.1.0/lib/axiom/relation.rb:109:in `each'
    from /home/aaron/.rvm/gems/ruby-1.9.3-p429@flatfiles/gems/axiom-0.1.0/lib/axiom/relation.rb:109:in `each'
    from /home/aaron/.rvm/gems/ruby-1.9.3-p429@flatfiles/gems/axiom-0.1.0/lib/axiom/algebra/restriction.rb:51:in `each'
    from axiom_test.rb:42:in `to_a'
    from axiom_test.rb:42:in `<main>'

This is obviously a contrived example. See (sort of) real use case here:

https://github.com/ahamid/flatfiles/blob/master/lib/flatfiles/record_file_enumerator.rb https://github.com/ahamid/flatfiles/blob/master/lib/flatfiles/veritas/tuple_provider.rb#L71

Is there really an assumption that the tuple enumerator represents a static, frozen content? I am a bit confused by how Adamantium is configured but Axiom::Relation appears to be inheriting Adamantium functionality via its inclusion of Equalizer:

https://github.com/dkubb/axiom/blob/master/lib/axiom/relation.rb#L8

(it appears that the module inclusion hook is called both for Equalizer and Relation)

dkubb commented 11 years ago

@ahamid Good catch. I meant for the underlying tuples to not be frozen. It's fine if they are, but I don't want to make any assumptions about the object passed in.

I'll look at how freezing is used in Axiom::Relation and see if I can be a bit less aggressive in freezing.

mbj commented 11 years ago

@dkubb Hah, we discussed this many times already. And I suggest you use Adamantium::Flat. I solved similar scenarios where "not my layers" data is reachable with "flat freezing".

dkubb commented 11 years ago

@mbj yeah, that's what I was thinking about doing.

Another thing I need to do is add some specs to methods to ensure the arguments passed in to most of my methods are not frozen as a side effect. If we need to freeze something, we should (deep) dup it and then freeze the clone.

dkubb commented 11 years ago

This was fixed in https://github.com/dkubb/axiom/pull/29