dkubb / axiom

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

Change Axiom::Relation to use Adamantium::Flat #29

Closed dkubb closed 11 years ago

dkubb commented 11 years ago

This should cause the object to be frozen, but it will not freeze any of it's dependencies.

dkubb commented 11 years ago

@ahamid does this fix the issue you reported in https://github.com/dkubb/axiom/issues/28 ?

ahamid commented 11 years ago

@dkubb I don't think so Dan. See #30 for spec

dkubb commented 11 years ago

@ahamid ok, I think I may have something now. I got your spec passing first, then I refactored the spec a bit (still checking to make sure it continued to fail when my fix was reversed).

Can you test it out and let me know if it fixed your problem?

ahamid commented 11 years ago

@dkubb Yes I think this fixes the problem (and this issue can probably be closed). It has allowed me to proceed, although I think I have run into another issue related to Tuple freezing. This may or may not be behavior that is intended to be supported by the library (I patch Axiom::Tuple to hold a reference to an internal structure which somehow in turn results in freezing a Class of mine and preventing new instances). I will write another spec to get your opinion. Thanks for looking into this.

dkubb commented 11 years ago

@ahamid Ok, I just merged this into master.

Yeah, Axiom::Tuple probably needs to deep freeze it's values. Maybe in another ticket you can describe what you're trying to do and we can work out an alternative that doesn't involve patching one of axiom's classes?

ahamid commented 11 years ago

Thanks @dkubb, freezing tuples does seem intuitively correct.

I'm using a framework that provides a DSL for record definitions, and I'm patching Axiom::Tuple in order to expose the underlying record (patching as opposed to extending because internal transforms convert to/from Axiom::Tuple). Yes, bad I know. In any case, the underlying @record somehow contains a backreference to its defining Class object, the Class itself gets frozen and then @record_class.new breaks. Debugging to figure out why and working on spec atm.

class Axiom::Tuple
  # exposes underlying @record
  include FlatFiles::Axiom::RecordTupleMixin
end

It may turn out in the end that what I'm doing is just something that is not philosophically supported by the lib and the answers is just stop doing that, I don't want to bend the library if my pet project is the only use case.

dkubb commented 11 years ago

@ahamid Another option may be to specify another class or a factory that creates something Axiom::Tuple-like. It could be a proxy object that adds it's own behaviour and then delegates down to an Axiom::Tuple instance when it doesn't understand something. Either way, I'll wait on your example and see if I can think of something that would be useful to the lib.

ahamid commented 11 years ago

Well it turns out I no longer need this behavior (access to underlying data associated on per-tuple basis), so that issue is moot.