Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 7 Support & Readme #685

Open
dmytro-strukov opened this issue Oct 7, 2024 · 8 comments
Open

Rails 7 Support & Readme #685

dmytro-strukov opened this issue Oct 7, 2024 · 8 comments

Comments

@dmytro-strukov
Copy link

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
Copy link

danielricecodes commented Oct 7, 2024

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
Copy link
Author

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

@kbrock
Copy link
Collaborator

kbrock commented Oct 9, 2024

@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
Copy link
Author

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.

  1. Run Migration to add constraint

@kbrock
Copy link
Collaborator

kbrock commented Oct 10, 2024

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
Copy link

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
Copy link

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
Copy link
Collaborator

kbrock commented Oct 24, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants