Hi folks, I''ve been working on an application that has separate notions of users and people, where: user belongs_to :person and person acts_as_nested_set (using BNS, of course) When Person is defined with (among other things): belongs_to :organization acts_as_nested_set :scope => :organization and I call Person.roots, I get this lovely error (which is very similar to the error I get if I call Person.destroy while using scoping): Mysql::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''? ? "organization_id IS NULL" : "organization_id = #{organization_id}" AND paren'' at line 1: SELECT * FROM people WHERE ((organization_id.nil? ? "organization_id IS NULL" : "organization_id = #{organization_id}" AND parent_id IS NULL)) ORDER BY lft Which is accompanied by this stack trace: K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/connection_adapters/abstract_adapter.rb:128:in `log'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/connection_adapters/mysql_adapter.rb:243:in `execute'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/connection_adapters/mysql_adapter.rb:399:in `select'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/connection_adapters/abstract/database_statements.rb:7:in `select_all'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/base.rb:427:in `find_by_sql'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/base.rb:997:in `find_every'' K:/Rails/InstantRails/ruby/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/ac tive_record/base.rb:418:in `find'' #{RAILS_ROOT}/vendor/plugins/better_nested_set/lib/better_nested_set.rb:128: in `roots'' #{RAILS_ROOT}/app/helpers/application_helper.rb:5:in `build_nested_tree'' #{RAILS_ROOT}/app/views/system/_users_box.rhtml:9:in `_run_rhtml_47app47views47system47_users_box46rhtml'' #{RAILS_ROOT}/app/views/system/manageusers.rhtml:1:in `_run_rhtml_47app47views47system47manageusers46rhtml'' -e:4:in `load'' -e:4 Houston, we have a problem! Does anyone have any ideas? For now, I can turn off the scoping during development, but scoping is a necessity for production. I''ve tried monkeying around with it a bit, and I''ve been able to get a bit closer, but I still wind up with an error on a SELECT statement that looks like this: Mysql::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '''' at line 1: SELECT * FROM people WHERE ((organization_id = #{organization_id} AND parent_id IS NULL)) ORDER BY lft This occurs when I change this line (line 79 in better_nested_set.rb) from A to B: A: options[:scope] = %(#{options[:scope].to_s}.nil? ? "#{options[:scope].to_s} IS NULL" : "#{options[:scope].to_s} = \#{#{options[:scope].to_s}}") B: if %(#{options[:scope].to_s}).nil? options[:scope] = "#{options[:scope].to_s} IS NULL" else options[:scope] = "#{options[:scope].to_s} \#{#{options[:scope].to_s}}" end The problem seems to be that the right side of the quoted comparison in the else portion of the above block doesn''t properly evaluate to organization_id in situ. I''ve tried a few different things to get it to work properly, but I can''t seem to figure out where it''d get a value for organization_id (i.e. the scope field). I''ll look at this again tomorrow, with fresh eyes, but so far, I''m at my wit''s end. At least I''ve made some progress, including trying to get the root and roots methods to evaluate the string (as that''s where I expect it should be happening), I just can''t seem to figure out how to make it fill in the blanks. Any advice would be welcome! I''d love to see a patch get committed, as it''ll make BNS better, and it''ll make my application work properly in production. Thanks, -Jon Mischo -------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/betternestedset-talk/attachments/20070605/315ad563/attachment-0001.html
Jean-Christophe Michel
2007-Jun-05 09:41 UTC
[Betternestedset-talk] Odd errors with scoping
Hi, Le 5 juin 07 ? 09:20, Jonathan Mischo a ?crit :> person acts_as_nested_set (using BNS, of course)Do you use edge ? http://opensource.symetrie.com/trac/better_nested_set/ticket/27 Class.roots has anyway a problem with scope. I could give the advice to use any = Class.find :first any.roots that should avoid the Class+scope problem Jean-Christophe Michel -- symetrie.com Better Nested Set for rails: http://opensource.symetrie.com/trac/better_nested_set
Jean-Christophe, I''m using stable as of last week. Using the method you outlined below still doesn''t work, though I do have two observations and have patched BNS to assume the below behaviors (yes, the patch is coming pre-commit to my own svn tree, which has all of this app''s plugins checked in, so that the app uses stable versions of all plugins): 1) Class.roots should not be scoped 2) Instance.roots should be scoped, but if we get rid of the evaluation in the assignment of acts_as_nested_set_options[:scope] and add a protected method to call in the instance methods that handles building the string, it should work. So here''s a patch where scoping works as indicated above, and [almost?] everything seems to work sensibly (you should probably play with it a bit to make sure things really work properly...they seem to be working when I poke and prod them, but I''m certainly not the expert on BNS, and I''m sure you could make things prettier): Index: K:/Rails/InstantRails/rails_apps/enkisuite/vendor/plugins/better_nested_set/ lib/better_nested_set.rb ==================================================================--- K:/Rails/InstantRails/rails_apps/enkisuite/vendor/plugins/better_nested_set/ lib/better_nested_set.rb (revision 21) +++ K:/Rails/InstantRails/rails_apps/enkisuite/vendor/plugins/better_nested_set/ lib/better_nested_set.rb (working copy) @@ -76,7 +76,7 @@ if options[:scope].to_s !~ /_id$/ options[:scope] = "#{options[:scope]}_id".intern end - options[:scope] = %(#{options[:scope].to_s}.nil? ? "#{options[:scope].to_s} IS NULL" : "#{options[:scope].to_s} \#{#{options[:scope].to_s}}") + #options[:scope] = %(#{options[:scope].to_s}.nil? ? "#{options[:scope].to_s} IS NULL" : "#{options[:scope].to_s} \#{#{options[:scope].to_s}}") end # options[:scope] = %("#{options[:scope]}") @@ -85,7 +85,7 @@ { :parent_column => (options[:parent_column] || ''parent_id''), :left_column => (options[:left_column] || ''lft''), :right_column => (options[:right_column] || ''rgt''), - :scope => (options[:scope] || ''1 = 1''), + :scope => (options[:scope] || ''''), :text_column => (options[:text_column] || columns.collect{|c| (c.type == :string) ? c.name : nil }.compact.first) } ) @@ -118,14 +118,14 @@ end - # Returns the single root + # Returns the first root def root - self.find(:first, :conditions => "(#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:parent_column]} IS NULL)") + self.find(:first, :conditions => "#{acts_as_nested_set_options[:parent_column]} IS NULL") end # Returns roots when multiple roots (or virtual root, which is the same) def roots - self.find(:all, :conditions => "(#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:parent_column]} IS NULL)", :order => "#{acts_as_nested_set_options[:left_column]}") + self.find(:all, :conditions => "#{acts_as_nested_set_options[:parent_column]} IS NULL", :order => "#{acts_as_nested_set_options[:left_column]}") end end @@ -136,7 +136,7 @@ # on creation, set automatically lft and rgt to the end of the tree def before_create - maxright self.class.maximum(acts_as_nested_set_options[:right_column], :conditions => acts_as_nested_set_options[:scope]) || 0 + maxright self.class.maximum(acts_as_nested_set_options[:right_column], :conditions => self.build_scoping_clause) || 0 # adds the new node to the right of all existing nodes self[acts_as_nested_set_options[:left_column]] = maxright+1 self[acts_as_nested_set_options[:right_column]] = maxright+2 @@ -199,8 +199,8 @@ child[acts_as_nested_set_options[:right_column]] right_bound + 1 self[acts_as_nested_set_options[:right_column]] += 2 self.class.transaction { - self.class.update_all( "#{acts_as_nested_set_options[:left_column]} (#{acts_as_nested_set_options[:left_column]} + 2)", "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:left_column]} >= #{right_bound}" ) - self.class.update_all( "#{acts_as_nested_set_options[:right_column]} (#{acts_as_nested_set_options[:right_column]} + 2)", "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:right_column]} >= #{right_bound}" ) + self.class.update_all( "#{acts_as_nested_set_options[:left_column]} (#{acts_as_nested_set_options[:left_column]} + 2)", self.build_scoping_clause + " AND #{acts_as_nested_set_options[:left_column]} >= #{right_bound}" ) + self.class.update_all( "#{acts_as_nested_set_options[:right_column]} (#{acts_as_nested_set_options[:right_column]} + 2)", self.build_scoping_clause + " AND #{acts_as_nested_set_options[:right_column]} >= #{right_bound}" ) self.save child.save } @@ -210,12 +210,12 @@ # Returns root def root - self.class.find(:first, :conditions => "#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:parent_column]} IS NULL)") + self.class.find(:first, :conditions => self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:parent_column]} IS NULL)") end # Returns roots when multiple roots (or virtual root, which is the same) def roots - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:parent_column]} IS NULL)", :order => "#{acts_as_nested_set_options[:left_column]}") + self.class.find(:all, :conditions => self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:parent_column]} IS NULL)", :order => "#{acts_as_nested_set_options[:left_column]}") end # Returns the parent @@ -226,7 +226,7 @@ # Returns an array of all parents # Maybe ''full_outline'' would be a better name, but we prefer to mimic the Tree class def ancestors - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:left_column]} < #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} > #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column] ) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:left_column]} < #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} > #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column] ) end # Returns the array of all parents and self @@ -244,15 +244,15 @@ if self[acts_as_nested_set_options[:parent_column]].nil? || self[acts_as_nested_set_options[:parent_column]].zero? [self] else - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} and #{acts_as_nested_set_options[:parent_column]} #{self[acts_as_nested_set_options[:parent_column]]}", :order => acts_as_nested_set_options[:left_column]) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND #{acts_as_nested_set_options[:parent_column]} #{self[acts_as_nested_set_options[:parent_column]]}", :order => acts_as_nested_set_options[:left_column]) end end # Returns the level of this object in the tree # root level is 0 def level - return 0 if self[acts_as_nested_set_options[:parent_column]].nil? - self.class.count("#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:left_column]} < #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} > #{self[acts_as_nested_set_options[:right_column]]})") + return 0 if self.parent.nil? + self.class.count(self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:left_column]} < #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} > #{self[acts_as_nested_set_options[:right_column]]})") end # Returns the number of nested children of this object. @@ -280,19 +280,19 @@ # get all subtrees and flatten the list exclude_list = special[:exclude].map{|e| e.full_set.map{|ee| ee.id}}.flatten.uniq.join('','') if exclude_list.blank? - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) else - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND id NOT IN (#{exclude_list}) AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND id NOT IN (#{exclude_list}) AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) end end else - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND (#{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]}) and (#{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]})", :order => acts_as_nested_set_options[:left_column]) end end # Returns a set of only this entry''s immediate children def children - self.class.find(:all, :conditions => "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:parent_column]} = #{self.id}", :order => acts_as_nested_set_options[:left_column]) + self.class.find(:all, :conditions => self.build_scoping_clause + " AND #{acts_as_nested_set_options[:parent_column]} = #{self.id}", :order => acts_as_nested_set_options[:left_column]) end # Prunes a branch off of the tree, shifting all of the elements on the right @@ -302,9 +302,9 @@ dif = self[acts_as_nested_set_options[:right_column]] - self[acts_as_nested_set_options[:left_column]] + 1 self.class.transaction { - self.class.delete_all( "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]}" ) - self.class.update_all( "#{acts_as_nested_set_options[:left_column]} (#{acts_as_nested_set_options[:left_column]} - #{dif})", "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:left_column]} >#{self[acts_as_nested_set_options[:right_column]]}" ) - self.class.update_all( "#{acts_as_nested_set_options[:right_column]} (#{acts_as_nested_set_options[:right_column]} - #{dif} )", "#{acts_as_nested_set_options[:scope]} AND #{acts_as_nested_set_options[:right_column]} >#{self[acts_as_nested_set_options[:right_column]]}" ) + self.class.delete_all( self.build_scoping_clause + " AND #{acts_as_nested_set_options[:left_column]} > #{self[acts_as_nested_set_options[:left_column]]} and #{acts_as_nested_set_options[:right_column]} < #{self[acts_as_nested_set_options[:right_column]]}" ) + self.class.update_all( "#{acts_as_nested_set_options[:left_column]} (#{acts_as_nested_set_options[:left_column]} - #{dif})", self.build_scoping_clause + " AND #{acts_as_nested_set_options[:left_column]} >#{self[acts_as_nested_set_options[:right_column]]}" ) + self.class.update_all( "#{acts_as_nested_set_options[:right_column]} (#{acts_as_nested_set_options[:right_column]} - #{dif} )", self.build_scoping_clause + " AND #{acts_as_nested_set_options[:right_column]} >#{self[acts_as_nested_set_options[:right_column]]}" ) } end @@ -324,6 +324,20 @@ end protected + def build_scoping_clause + unless "#{acts_as_nested_set_options[:scope]}" ="#{acts_as_nested_set_options[:parent_column]}" || "#{acts_as_nested_set_options[:scope]}".empty? + scoping_clause "#{acts_as_nested_set_options[:scope]} = " + unless self[acts_as_nested_set_options[:scope]].nil? + scoping_clause = scoping_clause + self[acts_as_nested_set_options[:scope]].to_s + else + scoping_clause = scoping_clause + "NULL" + end + else + scoping_clause = "true" + end + scoping_clause + end + def move_to(target, position) raise ActiveRecord::ActiveRecordError, "You cannot move a new node" if self.id.nil? @@ -406,7 +420,7 @@ WHEN #{self.class.primary_key} #{self.id} \ THEN #{new_parent} \ ELSE #{acts_as_nested_set_options[:parent_column]} END", - acts_as_nested_set_options[:scope] ) + self.build_scoping_clause ) self.reload end -----Original Message----- From: betternestedset-talk-bounces at rubyforge.org [mailto:betternestedset-talk-bounces at rubyforge.org] On Behalf Of Jean-Christophe Michel Sent: Tuesday, June 05, 2007 4:42 AM To: betternestedset-talk at rubyforge.org Subject: Re: [Betternestedset-talk] Odd errors with scoping Hi, Le 5 juin 07 ? 09:20, Jonathan Mischo a ?crit :> person acts_as_nested_set (using BNS, of course)Do you use edge ? http://opensource.symetrie.com/trac/better_nested_set/ticket/27 Class.roots has anyway a problem with scope. I could give the advice to use any = Class.find :first any.roots that should avoid the Class+scope problem Jean-Christophe Michel -- symetrie.com Better Nested Set for rails: http://opensource.symetrie.com/trac/better_nested_set _______________________________________________ Betternestedset-talk mailing list Betternestedset-talk at rubyforge.org http://rubyforge.org/mailman/listinfo/betternestedset-talk