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

Add workaround for crystal-lang/crystal#9483 #160

Merged
merged 2 commits into from
Jan 22, 2022

Conversation

bcardiff
Copy link
Member

After merging #159 I found and issue when using it in the actual drivers.

Building the specs of them leads to a:

BUG: trying to downcast DB::Transaction+ <- DB::TopLevelTransaction (Exception)
  ...

This seems related to crystal-lang/crystal#9483 which is still open.
The workaround is a safe cast before yielding. The cast is safe because of the return type restriction of begin_transaction : Transaction.

Unfortunately there is no way to add a spec for this on crystal-db because DummyDriver is loaded in the specs. DummyDriver defines its own custom transaction type and that, somehow, make the compile error go away also. Drivers do not need to define their own transaction.

By using a foo.cr in the root of crystal-db you can reproduce the issue, as long as DummyDriver is not loaded.

require "./src/db"

DB.open "fake://host" do |db|
  db.transaction do |tx_0|
    tx_0.transaction do |tx_1|
      tx_1.transaction do |tx_2|
      end
    end
    tx_0.rollback
  end
end

Note: there is no FakeDriver defined, and that is fine for this snippet. I was not able to reduce it further.

% crystal foo.cr
BUG: trying to downcast DB::Transaction+ <- DB::TopLevelTransaction (Exception)
  from raise<Exception>:NoReturn
  from raise<String>:NoReturn
  from Crystal::CodeGenVisitor#downcast_distinct<LLVM::Value, Crystal::Type+, Crystal::Type+>:NoReturn
  from Crystal::CodeGenVisitor#downcast<LLVM::Value, Crystal::Type+, Crystal::Type+, Bool>:LLVM::Value
  from Crystal::CodeGenVisitor#visit<Crystal::Var+>:(LLVM::Value | Nil)
  from Crystal::ASTNode+@Crystal::ASTNode#accept<Crystal::CodeGenVisitor>:Nil
  from Crystal::CodeGenVisitor#visit<Crystal::Call>:Bool
  from Crystal::ASTNode+@Crystal::ASTNode#accept<Crystal::CodeGenVisitor>:Nil

src/db/begin_transaction.cr Outdated Show resolved Hide resolved
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
@bcardiff bcardiff merged commit 27ade07 into master Jan 22, 2022
@bcardiff bcardiff deleted the fix/workaround-crystal-9483 branch January 27, 2022 13:58
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

Successfully merging this pull request may close these issues.

2 participants