JetBrains / Exposed

Kotlin SQL Framework
http://jetbrains.github.io/Exposed/
Apache License 2.0
8.25k stars 689 forks source link

Expression.toString needs to be connected to a database #1050

Open peter-plochan opened 4 years ago

peter-plochan commented 4 years ago

Expression#toString method tries to transitively call TransactionManager with connected database which makes toString pretty unusable mainly for unit testing frameworks where the DB connection is not really needed. See e.g. this stacktrace:

java.lang.IllegalStateException: Please call Database.connect() before using this code

    at org.jetbrains.exposed.sql.transactions.NotInitializedManager.currentOrNull(TransactionApi.kt:38)
    at org.jetbrains.exposed.sql.transactions.TransactionManager$Companion.currentOrNull(TransactionApi.kt:96)
    at org.jetbrains.exposed.sql.transactions.TransactionManager$Companion.current(TransactionApi.kt:98)
    at org.jetbrains.exposed.sql.Column.toQueryBuilder(Column.kt:41)
    at org.jetbrains.exposed.sql.QueryBuilder.append(Expression.kt:42)
    at org.jetbrains.exposed.sql.OpKt.appendExpression(Op.kt:544)
    at org.jetbrains.exposed.sql.OpKt.access$appendExpression(Op.kt:1)
    at org.jetbrains.exposed.sql.ComparisonOp$toQueryBuilder$1.invoke(Op.kt:130)
    at org.jetbrains.exposed.sql.ComparisonOp$toQueryBuilder$1.invoke(Op.kt:121)
    at org.jetbrains.exposed.sql.QueryBuilder.invoke(Expression.kt:17)
    at org.jetbrains.exposed.sql.ComparisonOp.toQueryBuilder(Op.kt:129)
    at org.jetbrains.exposed.sql.QueryBuilder.append(Expression.kt:42)
    at org.jetbrains.exposed.sql.Expression.toString(Expression.kt:131)
    at strikt.internal.reporting.FormattingKt.formatValue(Formatting.kt:44)
        ...
PeterAttardo commented 4 months ago

I'm running into this issue as well. Even if you connect to a database, it will throw with a different-but-related error (java.lang.IllegalStateException: No transaction in context.) if you try to call it outside of a transaction block.

It looks like Column itself overrides toString and uses a fairly straightforward expression: "${table.javaClass.name}.$name". However, anything that wraps a Column gets routed through Column's toQueryBuilder function, which uses TransactionManager's fullIdentity method.

Given that toString is already contextual (can yield different results when called in different transaction blocks, depending on the database they connect to), I see little downside in routing it to a default "generic SQL" IdentifierManagerApi when called from outside a transaction block with a specific known dialect.