igniterealtime/openfire-monitoring-plugin

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

Opened this issue · 7 comments

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!

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.

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... 🤔

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?

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) {

I absolutely see what you mean.

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

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)

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);
            }