stefankroes / ancestry

Organise ActiveRecord model into a tree structure
MIT License
3.73k stars 460 forks source link

Rails 7 Support & Readme #685

Open dmytro-strukov opened 2 weeks ago

dmytro-strukov commented 2 weeks ago

Hi there! Thank you for the gem, I wonder if it supports Rails 7.x. The second question, I read the README file and it seems counterintuitive on this part:

$ rails g migration add_[ancestry]_to_[table] ancestry:string:index
class AddAncestryToTable < ActiveRecord::Migration[6.1]
  def change
    change_table(:table) do |t|
      # postgres
      t.string "ancestry", collation: 'C', null: false
      t.index "ancestry"
      # mysql
      t.string "ancestry", collation: 'utf8mb4_bin', null: false
      t.index "ancestry"
    end
  end
end

Why do we have null: false on the ancestry column? How to create a root category for example that does not have any parent categories if this will create constraints on the DB level.

danielricecodes commented 2 weeks ago

I have the same questions. I am trying to add ancestry to an existing model with a lot of records, and you can not initialize a column as NOT NULL on a non empty table. How does one populate the ancestry column prior to setting the constraint?

dmytro-strukov commented 2 weeks ago

@kbrock Could you please clarify the question upper? I can create merge request to update README if it needed

kbrock commented 2 weeks ago

@dmytro-strukov thanks for the ping

So there are 2 modes:

# current mode:
Ancestry.default_ancestry_format = :materialized_path2

# legacy (and current default)
# not sure if we need to document this as new installs want to use the other
Ancestry.default_ancestry_format = :materialized_path 
# current / materialized_path2:
 t.string "ancestry", collation: 'C', null: false, default: "/"
# legacy  / materialized_path:
 t.string "ancestry", collation: 'C', null: true

Pretty sure the default: "/" is not necessary there but I included to make it more explicit and help explain. If you are having troubles with a nil, then I'm guessing you setup a materialized_path2 schema but did not configure the default_ancestry_format configuration parameter.

ASIDE:

Maybe the best way forward is change the gem's default for default_ancestry_format to the non-legacy materialized_path2 since that is what we want people to use going forward.

Extra docs:

materialized_path2: root_path: "/", child_path: "/1/, grand_path: "/1/2/"
materialized_path:  root: nil, child node: "1", grand_path: "1/2"

materialized_path2 is much easier to construct the paths in pure SQL, so it helps us transition more and more of our queries into pure sql. Though, that effort has been a bit slow as of late. Sorry.

dmytro-strukov commented 2 weeks ago

I have the same questions. I am trying to add ancestry to an existing model with a lot of records, and you can not initialize a column as NOT NULL on a non empty table. How does one populate the ancestry column prior to setting the constraint?

@danielricecodes I think you can try to separate steps to update your table: 1) At first add an ancestry column without constraints 2) After this in batches do:

YourModel.find_in_batches do |group|
   group.each { |record| record.save } # or record.update(ancestry: '/')
end

It will populate the ancestry column with "/" value, this logic is on the Model callbacks level.

3) Run Migration to add constraint

kbrock commented 2 weeks ago

Thanks @dmytro-strukov If you are adding a new table, you can probably just run a sql query in your migration to update them all.

  # add column no constraint
  YourModel.update_all(:ancestry => "/")
  # add constraint

Depending upon the table size, some people like to pull out of migrations and run the update in a script. And if you are running this while production is running, a batch size may help you. But the update_all should be pretty darn quick for a few hundred thousand rows.

danielricecodes commented 2 weeks ago

Thanks @dmytro-strukov If you are adding a new table, you can probably just run a sql query in your migration to update them all.


  # add column no constraint

  YourModel.update_all(:ancestry => "/")

  # add constraint

Depending upon the table size, some people like to pull out of migrations and run the update in a script. And if you are running this while production is running, a batch size may help you.

But the update_all should be pretty darn quick for a few hundred thousand rows.

I was coming back to say the same thing. Adding a default value to the ancestry field can quickly populate the field too - again - assuming the table isn't Big Data in size (< a few million rows). Folks will get into long running migrations if the table is huge and for that it's best to do an in_batches loop as @dmytro-strukov suggested.

andyundso commented 18 hours ago

I wonder if it supports Rails 7.x.

Just to mention it quickly, I did not test it on Rails 7.x, but the gem works with Rails 8 perfectly fine.

kbrock commented 13 hours ago

We are using rails 7.0 and upgrading to 7.1 and are having success with both. (though the ancestry version we are using is a little older)