search

r2dbc / r2dbc-mssql

R2DBC Driver for Microsoft SQL Server using TDS (Tabular Data Stream) Protocol
Apache License 2.0
183 stars 32 forks source link

Can't combine bind variables with T-SQL local variables #197

Closed lukaseder closed 3 years ago

lukaseder commented 3 years ago

Bug Report

Versions

Current Behavior

This seems to be perfectly valid in SQL Server:

System.out.println((
    Flux.from(connectionFactory.create())
        .flatMap(c -> c.createStatement("declare @i int = 1; select @i where @x = 1")
            .bind("x", 1)
            .execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
));

Yet, I'm getting:

Exception in thread "main" java.lang.IllegalStateException: No parameter binding for i
    at io.r2dbc.mssql.ParametrizedMssqlStatement$Bindings.validate(ParametrizedMssqlStatement.java:624)
    at io.r2dbc.mssql.ParametrizedMssqlStatement.execute(ParametrizedMssqlStatement.java:111)
    at io.r2dbc.mssql.ParametrizedMssqlStatement.execute(ParametrizedMssqlStatement.java:59)
    at org.jooq.testscripts.R2DBC.lambda$0(R2DBC.java:26)
    at reactor.core.publisher.FluxFlatMap$FlatMapMain.onNext(FluxFlatMap.java:386)
    at org.jooq.test.setup.DatabaseSetup$1.lambda$2(DatabaseSetup.java:293)
    at org.jooq.Publisher$1.onNext(Publisher.java:86)
    at reactor.core.publisher.StrictSubscriber.onNext(StrictSubscriber.java:89)
    at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1815)
    at reactor.core.publisher.MonoFlatMap$FlatMapInner.onNext(MonoFlatMap.java:249)
    at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
    at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1815)
    at reactor.core.publisher.MonoTakeLastOne$TakeLastOneSubscriber.onComplete(MonoTakeLastOne.java:119)
    at reactor.core.publisher.FluxMap$MapSubscriber.onComplete(FluxMap.java:142)
    at reactor.core.publisher.FluxFlatMap$FlatMapMain.checkTerminated(FluxFlatMap.java:846)
    at reactor.core.publisher.FluxFlatMap$FlatMapMain.drainLoop(FluxFlatMap.java:608)
    at reactor.core.publisher.FluxFlatMap$FlatMapMain.drain(FluxFlatMap.java:588)
    at reactor.core.publisher.FluxFlatMap$FlatMapMain.onComplete(FluxFlatMap.java:465)
    at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onComplete(FluxMapFuseable.java:150)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.checkTerminated(FluxWindowPredicate.java:527)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.drainLoop(FluxWindowPredicate.java:475)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.drain(FluxWindowPredicate.java:419)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.onComplete(FluxWindowPredicate.java:299)
    at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onComplete(FluxContextWrite.java:126)
    at io.r2dbc.mssql.util.FluxDiscardOnCancel$FluxDiscardOnCancelSubscriber.onComplete(FluxDiscardOnCancel.java:104)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onComplete(FluxHandle.java:212)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:133)
    at reactor.core.publisher.FluxPeekFuseable$PeekConditionalSubscriber.onNext(FluxPeekFuseable.java:854)
    at reactor.core.publisher.FluxPeekFuseable$PeekConditionalSubscriber.onNext(FluxPeekFuseable.java:854)
    at reactor.core.publisher.FluxPeekFuseable$PeekConditionalSubscriber.onNext(FluxPeekFuseable.java:854)
    at reactor.core.publisher.FluxHandle$HandleConditionalSubscriber.onNext(FluxHandle.java:326)
    at reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner.onNext(MonoFlatMapMany.java:250)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.EmitterProcessor.drain(EmitterProcessor.java:491)
    at reactor.core.publisher.EmitterProcessor.tryEmitNext(EmitterProcessor.java:299)
    at reactor.core.publisher.InternalManySink.emitNext(InternalManySink.java:27)
    at reactor.core.publisher.EmitterProcessor.onNext(EmitterProcessor.java:265)
    at io.r2dbc.mssql.client.ReactorNettyClient$1.next(ReactorNettyClient.java:244)
    at io.r2dbc.mssql.client.ReactorNettyClient$1.next(ReactorNettyClient.java:204)
    at io.r2dbc.mssql.message.token.Tabular$TabularDecoder.decode(Tabular.java:424)
    at io.r2dbc.mssql.client.ConnectionState$4$1.decode(ConnectionState.java:206)
    at io.r2dbc.mssql.client.StreamDecoder.withState(StreamDecoder.java:137)
    at io.r2dbc.mssql.client.StreamDecoder.decode(StreamDecoder.java:109)
    at io.r2dbc.mssql.client.ReactorNettyClient.lambda$new$6(ReactorNettyClient.java:254)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:184)
    at reactor.netty.channel.FluxReceive.drainReceiver(FluxReceive.java:265)
    at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:371)
    at reactor.netty.channel.ChannelOperations.onInboundNext(ChannelOperations.java:358)
    at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:96)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
    at io.r2dbc.mssql.client.ssl.TdsSslHandler.channelRead(TdsSslHandler.java:380)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Thread.java:834)
    Suppressed: java.lang.Exception: #block terminated with an error
        at reactor.core.publisher.BlockingSingleSubscriber.blockingGet(BlockingSingleSubscriber.java:99)
        at reactor.core.publisher.Mono.block(Mono.java:1703)

Expected behavior/code

The driver should be able to distinguish between local variables and bind parameters. JDBC doesn't have this problem as JDBC uses ? for parameter markers.

Possible Solution

A workaround is to avoid bind variables as soon as local variables are being used:

System.out.println((
    Flux.from(connectionFactory.create())
        .flatMap(c -> c.createStatement("declare @i int = 1; select @i where 1 = 1")
            .execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
));

It's hard to say how else this could be solved in the driver. A heuristic would regex-search for declare\s+@(\w+) and exclude those names from being required as bind variables. An actual parser would be even better, but not really practical, given T-SQL's vast syntax.

Also, this shows that a solution shouldn't just stop requiring @i from being bound, but even prevent it from being bound. This is also illegal in SQL Server:

System.out.println((
    Flux.from(connectionFactory.create())
        .flatMap(c -> c.createStatement("declare @i int = 1; select @i where @x = 1")
            .bind("x", 1)
            .bind("i", 2)
            .execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
));

Leading to:

Exception in thread "main" io.r2dbc.mssql.ExceptionFactory$MssqlBadGrammarException: [134] [S0001] The variable name '@i' has already been declared. Variable names must be unique within a query batch or stored procedure.
    at io.r2dbc.mssql.ExceptionFactory.createException(ExceptionFactory.java:144)
    at io.r2dbc.mssql.MssqlResult.lambda$map$2(MssqlResult.java:179)
    at reactor.core.publisher.FluxHandleFuseable$HandleFuseableSubscriber.onNext(FluxHandleFuseable.java:169)
    at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onNext(FluxContextWrite.java:107)
    at io.r2dbc.mssql.util.FluxDiscardOnCancel$FluxDiscardOnCancelSubscriber.onNext(FluxDiscardOnCancel.java:89)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.FluxFilter$FilterSubscriber.onNext(FluxFilter.java:113)
    at reactor.core.publisher.FluxHandle$HandleConditionalSubscriber.onNext(FluxHandle.java:326)
    at reactor.core.publisher.EmitterProcessor.drain(EmitterProcessor.java:491)
    at reactor.core.publisher.EmitterProcessor.tryEmitNext(EmitterProcessor.java:299)
    at reactor.core.publisher.InternalManySink.emitNext(InternalManySink.java:27)
    at reactor.core.publisher.EmitterProcessor.onNext(EmitterProcessor.java:265)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:118)
    at reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner.onNext(MonoFlatMapMany.java:250)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:199)
    at reactor.core.publisher.EmitterProcessor.drain(EmitterProcessor.java:491)
    at reactor.core.publisher.EmitterProcessor.tryEmitNext(EmitterProcessor.java:299)
    at reactor.core.publisher.InternalManySink.emitNext(InternalManySink.java:27)
    at reactor.core.publisher.EmitterProcessor.onNext(EmitterProcessor.java:265)
    at io.r2dbc.mssql.client.ReactorNettyClient$1.next(ReactorNettyClient.java:244)
    at io.r2dbc.mssql.client.ReactorNettyClient$1.next(ReactorNettyClient.java:204)
    at io.r2dbc.mssql.message.token.Tabular$TabularDecoder.decode(Tabular.java:424)
    at io.r2dbc.mssql.client.ConnectionState$4$1.decode(ConnectionState.java:206)
    at io.r2dbc.mssql.client.StreamDecoder.withState(StreamDecoder.java:137)
    at io.r2dbc.mssql.client.StreamDecoder.decode(StreamDecoder.java:109)
    at io.r2dbc.mssql.client.ReactorNettyClient.lambda$new$6(ReactorNettyClient.java:254)
    at reactor.core.publisher.FluxPeek$PeekSubscriber.onNext(FluxPeek.java:184)
    at reactor.netty.channel.FluxReceive.drainReceiver(FluxReceive.java:265)
    at reactor.netty.channel.FluxReceive.onInboundNext(FluxReceive.java:371)
    at reactor.netty.channel.ChannelOperations.onInboundNext(ChannelOperations.java:358)
    at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:96)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
    at io.r2dbc.mssql.client.ssl.TdsSslHandler.channelRead(TdsSslHandler.java:380)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Thread.java:834)
    Suppressed: java.lang.Exception: #block terminated with an error
        at reactor.core.publisher.BlockingSingleSubscriber.blockingGet(BlockingSingleSubscriber.java:99)
        at reactor.core.publisher.Mono.block(Mono.java:1703)
        at org.jooq.testscripts.R2DBC.main(R2DBC.java:30)
lukaseder commented 3 years ago

Thanks for the fix. I agree that letting the server validate these variables is a pragmatic approach that should work well.

mp911de commented 3 years ago

Snapshots are available from oss.sonatype.org for 0.8.6 and 0.9.0 versions.

lukaseder commented 3 years ago

I can confirm, jOOQ's INSERT/UPDATE/DELETE .. RETURNING emulations now work correctly on SQL Server