igniterealtime / openfire-monitoring-plugin

Adds support for chat archiving and server statistics to Openfire
Apache License 2.0
18 stars 28 forks source link

SQL Server error: An expression of non-boolean type specified in a context where a condition is expected, near 'RowNum' #363

Open UHolder opened 1 year ago

UHolder commented 1 year ago

Openfire 4.7.5, build ee4395e Monitoring Service Plugin 2.5.0

Microsoft SQL Server 2008 R2 (SP3-GDR) (KB4057113) - 10.50.6560.0 (X64) Dec 28 2017 15:03:48 Copyright (c) Microsoft Corporation Standard Edition (64-bit) on Windows NT 6.1 (Build 7601: Service Pack 1)

There's a common SQL Server error in the Openfire logs: An expression of non-boolean type specified in a context where a condition is expected, near 'RowNum':

2023.09.06 12:49:31 ERROR [socket_c2s-thread-26]: com.reucon.openfire.plugin.archive.impl.JdbcPersistenceManager - Error selecting conversations
com.microsoft.sqlserver.jdbc.SQLServerException: An expression of non-boolean type specified in a context where a condition is expected, near 'RowNum'.
    at com.microsoft.sqlserver.jdbc.SQLServerException.makeFromDatabaseError(SQLServerException.java:265) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.getNextResult(SQLServerStatement.java:1662) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement.doExecutePreparedStatement(SQLServerPreparedStatement.java:615) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement$PrepStmtExecCmd.doExecute(SQLServerPreparedStatement.java:537) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.TDSCommand.execute(IOBuffer.java:7417) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerConnection.executeCommand(SQLServerConnection.java:3488) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeCommand(SQLServerStatement.java:262) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerStatement.executeStatement(SQLServerStatement.java:237) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement.executeQuery(SQLServerPreparedStatement.java:456) ~[mssql-jdbc-9.4.1.jre8.jar:?]
    at org.apache.commons.dbcp2.DelegatingPreparedStatement.executeQuery(DelegatingPreparedStatement.java:122) ~[commons-dbcp2-2.9.0.jar:2.9.0]
    at org.apache.commons.dbcp2.DelegatingPreparedStatement.executeQuery(DelegatingPreparedStatement.java:122) ~[commons-dbcp2-2.9.0.jar:2.9.0]
    at com.reucon.openfire.plugin.archive.impl.JdbcPersistenceManager.findConversations(JdbcPersistenceManager.java:162) [monitoring-2.5.0.jar!/:?]
    at com.reucon.openfire.plugin.archive.xep0136.IQListHandler.list(IQListHandler.java:57) [monitoring-2.5.0.jar!/:?]
    at com.reucon.openfire.plugin.archive.xep0136.IQListHandler.handleIQ(IQListHandler.java:41) [monitoring-2.5.0.jar!/:?]
    at com.reucon.openfire.plugin.archive.xep.AbstractXepSupport$1.handleIQ(AbstractXepSupport.java:49) [monitoring-2.5.0.jar!/:?]
    at org.jivesoftware.openfire.handler.IQHandler.process(IQHandler.java:62) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.IQRouter.handle(IQRouter.java:389) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.IQRouter.route(IQRouter.java:105) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.spi.PacketRouterImpl.route(PacketRouterImpl.java:74) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.net.StanzaHandler.processIQ(StanzaHandler.java:369) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.net.ClientStanzaHandler.processIQ(ClientStanzaHandler.java:95) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.net.StanzaHandler.process(StanzaHandler.java:311) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.net.StanzaHandler.process(StanzaHandler.java:198) [xmppserver-4.7.5.jar:4.7.5]
    at org.jivesoftware.openfire.nio.ConnectionHandler.messageReceived(ConnectionHandler.java:183) [xmppserver-4.7.5.jar:4.7.5]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain$TailFilter.messageReceived(DefaultIoFilterChain.java:1015) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:49) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:1128) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.IoFilterAdapter.messageReceived(IoFilterAdapter.java:122) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:49) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:1128) [mina-core-2.1.3.jar:?]
    at org.apache.mina.filter.codec.ProtocolCodecFilter$ProtocolDecoderOutputImpl.flush(ProtocolCodecFilter.java:413) [mina-core-2.1.3.jar:?]
    at org.apache.mina.filter.codec.ProtocolCodecFilter.messageReceived(ProtocolCodecFilter.java:257) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:49) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:1128) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.filterchain.IoFilterEvent.fire(IoFilterEvent.java:106) [mina-core-2.1.3.jar:?]
    at org.apache.mina.core.session.IoEvent.run(IoEvent.java:89) [mina-core-2.1.3.jar:?]
    at org.apache.mina.filter.executor.OrderedThreadPoolExecutor$Worker.runTask(OrderedThreadPoolExecutor.java:766) [mina-core-2.1.3.jar:?]
    at org.apache.mina.filter.executor.OrderedThreadPoolExecutor$Worker.runTasks(OrderedThreadPoolExecutor.java:758) [mina-core-2.1.3.jar:?]
    at org.apache.mina.filter.executor.OrderedThreadPoolExecutor$Worker.run(OrderedThreadPoolExecutor.java:697) [mina-core-2.1.3.jar:?]
    at java.lang.Thread.run(Thread.java:750) [?:1.8.0_382] 

Here's example of the wrong query with mistake in last WHERE clause:

SELECT *
FROM (
        SELECT  *, 
                ROW_NUMBER() OVER (ORDER BY ofConversation.conversationID) AS RowNum 
        FROM ( 
                SELECT ofConversation.conversationID, ofConversation.room, ofConversation.isExternal, 
                        ofConversation.lastActivity, ofConversation.messageCount, ofConversation.startDate, 
                        ofConParticipant.bareJID, ofConParticipant.jidResource, ofConParticipant.nickname, 
                        ofConParticipant.bareJID AS fromJID, ofConParticipant.jidResource AS fromJIDResource, 
                        ofMessageArchive.toJID, ofMessageArchive.toJIDResource, 
                        min(ofConParticipant.joinedDate) AS joinedDate, 
                        max(ofConParticipant.leftDate) as leftDate 
                FROM ofConversation 
                        INNER JOIN ofConParticipant ON ofConversation.conversationID = ofConParticipant.conversationID 
                        INNER JOIN (
                                    SELECT conversationID, toJID, toJIDResource 
                                    FROM ofMessageArchive 
                                    union all 
                                    SELECT conversationID, fromJID as toJID, fromJIDResource as toJIDResource 
                                    FROM ofMessageArchive
                                   ) ofMessageArchive ON ofConParticipant.conversationID = ofMessageArchive.conversationID  
                WHERE ofConParticipant.bareJID = N'user1@domain.com' AND ofMessageArchive.toJID = N'user2@domain.com'
                GROUP BY ofConversation.conversationID, ofConversation.room, ofConversation.isExternal, ofConversation.lastActivity, ofConversation.messageCount, ofConversation.startDate, ofConParticipant.bareJID, ofConParticipant.jidResource, ofConParticipant.nickname, ofConParticipant.bareJID, ofMessageArchive.toJID, ofMessageArchive.toJIDResource
            ) ofConversation 
    ) t2 
WHERE RowNum

Seems it's some bug in JdbcPersistenceManager.java if DatabaseType.sqlserver.

What does this error affect?

Please fix the bug in the next release. Thanks in advance!

Fishbowler commented 1 year ago

I tried reproducing this in a containerised environment.

Using the sqlserver folder of https://github.com/Fishbowler/openfire-docker-compose/tree/b1d0b0a18c9d220cffb0497adeb54bedddbad989 and the current Openfire 4.8 nightly, I can't reproduce it.

I note the IQListHandler in the stacktrace, so have been using stanzas such as

<iq type='get' id='issue363'>
  <list xmlns='urn:xmpp:archive'>
    <set xmlns='http://jabber.org/protocol/rsm'>
      <max>30</max>
    </set>
  </list>
</iq>

There are differences in setup (OS, Openfire version, SQL Server version). OS shouldn't ever make a difference, Openfire version shouldn't make a difference here (since this is all Monitoring plugin code) but SQL Server version might... except I can't run 2008 R2 in a container, and I don't have a Windows lab to play with.

It would be really handy if there's any context e.g. surrounding logs that give some insight into what's inbound to the server at this point.

Hoping it's not as trivial as WHERE RowNum isn't allowed in 2008 R2. That line's unchanged in ~9 years.

Fishbowler commented 1 year ago

Although....

Using https://sqltest.net/ running a query like this:

SELECT * 
  FROM sql_server_test_a
  where id

gives exactly the same error as above. This claims to be running (emulating?) SQL Server 2019. Although so was I... :thinking:

Fishbowler commented 1 year ago

Looking at the query, that ofConversation near the end of the query is rather suspicious. Would that redeclare what ofConversation is in the current query context?

UHolder commented 1 year ago

Error occurs after sending or receiving a message.

Will try to help to find the problem faster. Let look at the code. Something is wrong in JdbcPersistenceManager.java. Attention to lines 146-152, query generation block:

if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver) {
    querySB.insert(0,"SELECT * FROM (SELECT *, ROW_NUMBER() OVER (ORDER BY "+CONVERSATION_ID+") AS RowNum FROM ( ");
    querySB.append(") ofConversation ) t2 WHERE RowNum");
}
else {
    querySB.append(" ORDER BY ").append(CONVERSATION_ID);
}

The if clause here makes sense because SQL Server does not support LIMIT (and OFFSET support available in later versions only). So wrapper to the final query with row numbering to implement RowNum BETWEEN ... instead of OFFSET is correct (for DatabaseType.sqlserver only).

And this is the part that gives such result at the end: WHERE RowNum

Next we need to finish condition for DatabaseType.sqlserver by adding BETWEN ... AND. This implemented correctly also in lines 131-138:

if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver) {
    limitSB.append(" BETWEEN ").append(firstIndex+1);
    limitSB.append(" AND ").append(firstIndex+max);
}
else {
    limitSB.append(" LIMIT ").append(max);
    limitSB.append(" OFFSET ").append(firstIndex);
}

And then limitSB appends to the result query in line 153: querySB.append(limitSB);

The problem is that in some cases limitSB (at least for DatabaseType.sqlserver) contains empty text. Please note: limitSB implemented in another conditional brackets, lines 112-140:

if (xmppResultSet != null) {
...
    if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver) {
        limitSB.append(" BETWEEN ").append(firstIndex+1);
        limitSB.append(" AND ").append(firstIndex+max);
    }
    else {
        limitSB.append(" LIMIT ").append(max);
        limitSB.append(" OFFSET ").append(firstIndex);
    }
...
}

But adding RowNum to the query for DatabaseType.sqlserver is unambiguous (!). That is the problem I think.

I'm not a java expert, so I can't make changes to the code here, but I think it's possible to fix the error, for example, like this: Replace line 146 with if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver && xmppResultSet != null) { instead of if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver) {

Fishbowler commented 12 months ago

I absolutely see what you mean.

If xmppResultSet is null, then there's WHERE RowNum without the end of the clause.

Fishbowler commented 12 months ago

I've been able to reproduce this in my local SQLServer environment by making queries to empty archives (e.g. with a newly created user)

Fishbowler commented 12 months ago

There - got a fix PR open that works on my setup. Thanks for your help with this.

I opted to move the entire WHERE bit together, I felt like it made sense with the non-sqlserver bit below it:

            if (DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.sqlserver) {
                limitSB.append(" WHERE RowNum BETWEEN ").append(firstIndex+1);
                limitSB.append(" AND ").append(firstIndex+max);
            }
            else {
                limitSB.append(" LIMIT ").append(max);
                limitSB.append(" OFFSET ").append(firstIndex);
            }