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.

Sign up for GitHub

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

Jump to bottom

ActiveRecord::ConnectionAdapters::SQLite3Adapter#initialize does not correctly create missing parent directories #51623

Closed
postmodern opened this issue Apr 21, 2024 · 6 comments · Fixed by #51628
Closed

ActiveRecord::ConnectionAdapters::SQLite3Adapter#initialize does not correctly create missing parent directories #51623

postmodern opened this issue Apr 21, 2024 · 6 comments · Fixed by #51628
Labels
activerecord SQLite

Comments

@postmodern
Copy link
Contributor

ActiveRecord::ConnectionAdapters::SQLite3Adapter#initialize will check if a sqlite3 database file path exists, and if not it will attempt to create the directory using Dir.mkdir. However, Dir.mkdir cannot create all parent directories like FileUtils.mkdir_p. Thus an No such file or directory @ dir_s_mkdir - does/not/exist/yet (Errno::ENOENT) will be raised if one of the parent-parent directories of the sqlite3 database file do not exist yet.

Steps to reproduce

  1. ActiveRecord::Base.establish_connect({adapter: 'sqlite3', database: 'does/not/exist/yet/database.sqlite3'})
  2. migrations = ActiveRecord::MigrationContext.new(['path/to/db/migrate'])

Your reproduction script goes here

require 'active_record'

ActiveRecord::Base.establish_connect({adapter: 'sqlite3', database: 'does/not/exist/yet/database.sqlite3'})
migrations = ActiveRecord::MigrationContext.new(['path/to/db/migrate'])

Expected behavior

Use FileUtils.mkdir_p to create all parent directories for the missing sqlite3 database file.

Actual behavior

/home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:119:in `rescue in initialize': Database not found (ActiveRecord::NoDatabaseError)
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:115:in `initialize'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:24:in `new'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:24:in `sqlite3_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `public_send'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:723:in `checkout_new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:702:in `try_to_checkout_new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:654:in `acquire_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:353:in `checkout'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:182:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_handler.rb:246:in `retrieve_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_handling.rb:287:in `retrieve_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_handling.rb:254:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/tasks/database_tasks.rb:510:in `migration_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/migration.rb:1367:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/migration.rb:1229:in `initialize'
	... 5 levels...
/home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:116:in `mkdir': No such file or directory @ dir_s_mkdir - does/not/exist/yet (Errno::ENOENT)
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:116:in `initialize'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:24:in `new'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:24:in `sqlite3_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `public_send'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:676:in `new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:723:in `checkout_new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:702:in `try_to_checkout_new_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:654:in `acquire_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:353:in `checkout'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_pool.rb:182:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_adapters/abstract/connection_handler.rb:246:in `retrieve_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_handling.rb:287:in `retrieve_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/connection_handling.rb:254:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/tasks/database_tasks.rb:510:in `migration_connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/migration.rb:1367:in `connection'
	from /home/postmodern/.gem/ruby/gems/activerecord-7.1.3.2/lib/active_record/migration.rb:1229:in `initialize'
	... 5 levels...

System configuration

Rails version: 7.1.3.2

Ruby version: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]
@maniSHarma7575
Copy link
Contributor

@postmodern Please check this one. Seems like the actual cause of the issue. #50391

@postmodern
Copy link
Contributor Author

@maniSHarma7575 as indicated in my issue, I'm not using rails db:drop. I am directly calling ActiveRecord::Base.establish_connection and directly invoking the migrations using ActiveRecord::MigrationContext.new(['path/to/db/migrate']). I discovered this limitation when trying to run my own database tool that uses ActiveRecord to create a database in the ~/.local/share/ronin-db/ directory, but when I ran it in a completely new user environment that did not have a ~/.local directory, ActiveRecord failed.

@maniSHarma7575
Copy link
Contributor

@maniSHarma7575 as indicated in my issue, I'm not using rails db:drop. I am directly calling ActiveRecord::Base.establish_connection and directly invoking the migrations using ActiveRecord::MigrationContext.new(['path/to/db/migrate']). I discovered this limitation when trying to run my own database tool that uses ActiveRecord to create a database in the ~/.local/share/ronin-db/ directory, but when I ran it in a completely new user environment that did not have a ~/.local directory, ActiveRecord failed.

@postmodern I think it's intentional that it should raise an error when parent directory does not exists. Rails have a test case for that in source code as well.

rails/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb

Lines 26 to 32 in 82054e8

def test_bad_connection
error = assert_raise ActiveRecord::NoDatabaseError do
connection = SQLite3Adapter.new(adapter: "sqlite3", database: "/tmp/should/_not/_exist/-cinco-dog.db")
connection.drop_table "ex", if_exists: true
end
assert_kind_of ActiveRecord::ConnectionAdapters::NullPool, error.connection_pool
end

@maniSHarma7575
Copy link
Contributor

@eileencodes Should we fixed this? For Sqlite3 Rails should allow to create the database even if the database path with parent directories doesn't exists?

@postmodern
Copy link
Contributor Author

postmodern commented Apr 22, 2024 •
edited

@maniSHarma7575 why? It does not have to raise an error, it can simply create the parent directories using FileUtils.mkdir_p. I don't see any benefit for having such a limitation. This only prevents ActiveRecord from creating databases in nested directories that do not exist yet.

@maniSHarma7575 maniSHarma7575 mentioned this issue Apr 22, 2024
Merged
4 tasks
@maniSHarma7575
Copy link
Contributor

@maniSHarma7575 why? It does not have to raise an error, it can simply create the parent directories using FileUtils.mkdir_p. I don't see any benefit for having such a limitation. This only prevents ActiveRecord from creating databases in nested directories that do not exist yet.

@postmodern I also agree with you and have created a pull request for it. However, I'm unsure about the opinion of the Rails core team. Let's wait for their input and see what their take on it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
activerecord SQLite
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[FIX] Sqlite3 Should create db for missing parent directories as well maniSHarma7575/rails
3 participants
@postmodern @yahonda @maniSHarma7575