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

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

Closed
lukaseder opened this issue Jun 28, 2021 · 6 comments
Closed

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

lukaseder opened this issue Jun 28, 2021 · 6 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@lukaseder
Copy link

Bug Report

Versions

  • Driver: r2dbc-mssql 0.9.0.BUILD-SNAPSHOT
  • Database: Microsoft SQL Server 2019 (RTM-CU8-GDR) (KB4583459) - 15.0.4083.2 (X64) Nov 2 2020 18:35:09 Copyright (C) 2019 Microsoft Corporation Express Edition (64-bit) on Linux (Ubuntu 18.04.5 LTS)
  • Java: openjdk version "17-ea" 2021-09-14 OpenJDK Runtime Environment (build 17-ea+17-1401) OpenJDK 64-Bit Server VM (build 17-ea+17-1401, mixed mode, sharing)
  • OS: Microsoft Windows [Version 10.0.19042.1052]

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 mp911de added the type: enhancement A general enhancement label Jun 28, 2021
@mp911de
Copy link
Member

mp911de commented Jun 28, 2021

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

@lukaseder
Copy link
Author

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 mp911de added status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged and removed type: enhancement A general enhancement labels Jul 19, 2021
@mp911de
Copy link
Member

mp911de commented Jul 19, 2021

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)

mp911de added a commit that referenced this issue Jul 19, 2021
[#209]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
mp911de added a commit that referenced this issue Jul 19, 2021
[#209]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
@lukaseder
Copy link
Author

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
Copy link
Member

mp911de commented Jul 19, 2021

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
Copy link
Author

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.

@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Nov 9, 2021
@mp911de mp911de added this to the 0.8.8.RELEASE milestone Nov 9, 2021
@mp911de mp911de closed this as completed in 6b9f927 Nov 9, 2021
mp911de added a commit that referenced this issue Nov 9, 2021
We now no longer prefer cursor usage by default when the statement ends with a FOR XML/JSON clause to avoid server side errors reporting that cursor usage is not supported.

[resolves #209]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants