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

Default connection always used for internal transaction operations #229

Open
andrewMacmurray opened this issue Sep 2, 2024 · 0 comments

Comments

@andrewMacmurray
Copy link

andrewMacmurray commented Sep 2, 2024

We're using clear in a monorepo for a number of crystal services which connect to different databases.

We noticed that if you register just named connections (and not the default) calling any create / save operation clear throws an error:

You're trying to access the connection default which is not initialized

....

from src/clear/sql/connection_pool.cr:16:44 in 'save'
from src/clear/model/modules/has_saving.cr:148:55 in 'save!'
from example.cr:8:3 in 'create!'
from example.cr:18:1 in '__crystal_main'
from /Users/andrewmacmurray/Projects/concentric/libs/crystal-1.12.2-1/src/crystal/main.cr:129:5 in 'main_user_code'
from /Users/andrewmacmurray/Projects/concentric/libs/crystal-1.12.2-1/src/crystal/main.cr:115:7 in 'main'
from /Users/andrewmacmurray/Projects/concentric/libs/crystal-1.12.2-1/src/crystal/main.cr:141:3 in 'main'

Internally save! uses a transaction which doesn't pass the model connection name, so the query gets executed on the default connection.

This is a minimal example of the issue (run with crystal run example.cr):

# example.cr
require "./src/clear"

# Setup DB
system("dropdb posts_db --if-exists")
system("createdb posts_db")
system("echo \"CREATE TABLE post_stats (id serial PRIMARY KEY, post_id INTEGER);\" | psql posts_db 1>/dev/null")

class PostStats
  include Clear::Model

  self.connection = "postsdb"

  column id : Int32, primary: true, presence: false
  column post_id : Int32
end

# Register a single named connection
Clear::SQL.init("postsdb", "postgres://localhost/posts_db")

# Throws an exception
pp PostStats.create!({post_id: 1})

There's a fix in this commit that passes through the connection internally to transactions.

Additionally the model connection is not passed for aggregation queries and Collection.delete_all

# Both these throw errors
PostStats.query.count
PostStats.query.delete_all

There's a separate fix in this commit for aggregations and delete_all.

I didn't open a PR as I wasn't sure about the best way to test these changes - the tests depend on a default connection so it's difficult to reproduce that error. Happy to submit a PR though if you have any thoughts.

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

1 participant