anykeyh/clear

Default connection always used for internal transaction operations

Opened this issue · 0 comments

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.