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

The FOR XML clause is not allowed in a CURSOR statement #209

Closed lukaseder closed 2 years ago

lukaseder commented 3 years ago

Bug Report

Versions

Current Behavior

Run a query containing the T-SQL FOR clause with R2DBC and get this error:

Exception in thread "main" io.r2dbc.mssql.ExceptionFactory$MssqlNonTransientException: [6819] [S0002] The FOR XML clause is not allowed in a CURSOR statement.
    at io.r2dbc.mssql.ExceptionFactory.createException(ExceptionFactory.java:152)
    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.FluxWindowPredicate$WindowFlux.drainRegular(FluxWindowPredicate.java:657)
    at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drain(FluxWindowPredicate.java:735)
    at reactor.core.publisher.FluxWindowPredicate$WindowFlux.onNext(FluxWindowPredicate.java:777)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.onNext(FluxWindowPredicate.java:255)
    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.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:831)
    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:42)

Steps to reproduce

System.out.println(
    Flux.from(cf.create())
        .flatMap(c -> c.createStatement("select 1 as a for xml path").execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
    );

Expected behavior/code

This should work out of the box, just like with JDBC and produce:

<row><a>1</a></row>
mp911de commented 3 years ago

This is somewhat related to #66 as we don't have support for XML yet.

lukaseder commented 3 years ago

Maybe. This works:

System.out.println(
    Flux.from(cf.create())
        .flatMap(c -> c.createStatement("select (select 1 as a for xml path)").execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
    );

The result is produced as String. This also doesn't work:

System.out.println(
    Flux.from(cf.create())
        .flatMap(c -> c.createStatement("select 1 as a for json path").execute())
        .flatMap(it -> it.map((r, m) -> r.get(0)))
        .collectList()
        .block()
    );
mp911de commented 3 years ago

The issue is related to the default cursor usage in the driver. Setting fetchSize to zero bypasses cursor usage. With JDBC, I'm able to reproduce the issue via:

Statement statement = c.createStatement(ResultSet.TYPE_SCROLL_SENSITIVE, ResultSet.CONCUR_READ_ONLY);
ResultSet resultSet = statement.executeQuery("select 1 as a for xml path");

resulting in

Caused by: com.microsoft.sqlserver.jdbc.SQLServerException: The FOR XML clause is not allowed in a CURSOR statement.
    at com.microsoft.sqlserver.jdbc.SQLServerException.makeFromDatabaseError(SQLServerException.java:262)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.getNextResult(SQLServerStatement.java:1632)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.doExecuteCursored(SQLServerStatement.java:2030)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.doExecuteStatement(SQLServerStatement.java:846)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement$StmtExecCmd.doExecute(SQLServerStatement.java:767)
    at com.microsoft.sqlserver.jdbc.TDSCommand.execute(IOBuffer.java:7418)
    at com.microsoft.sqlserver.jdbc.SQLServerConnection.executeCommand(SQLServerConnection.java:3274)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeCommand(SQLServerStatement.java:247)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeStatement(SQLServerStatement.java:222)
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeQuery(SQLServerStatement.java:692)
    at com.zaxxer.hikari.pool.ProxyStatement.executeQuery(ProxyStatement.java:110)
    at com.zaxxer.hikari.pool.HikariProxyStatement.executeQuery(HikariProxyStatement.java)
lukaseder commented 3 years ago

I see, I hadn't thought of that possibility. But the equivalent of JDBC's ResultSet.TYPE_SCROLL_SENSITIVE is not necesssary in this case, as a default, right?

mp911de commented 3 years ago

It's not required as we assume by default forward-only cursors and setting the fetch size is the main indicator for the cursor page size. Setting the fetch size to zero expresses the intent to fetch all data directly and so we bypass the cursor.

Users can configure some defaults, whether to prefer cursored execution or if the statement starts with SELECT to apply application-specific customizations, but for a client library, it makes sense to set fetch size to zero when consuming results as JSON/XML.

lukaseder commented 3 years ago

Could there be a more explicit setting to govern this behaviour? I don't like the idea of setting the fetch size on behalf of the user too much, even if it's only for these FOR XML / FOR JSON queries, in case of which the fetch size doesn't matter since everything is aggregated in a single value... As a last resort, that would be doable, of course, but it seems like an accidental side effect.