From bc0caf591bfdb89c8fd75229abf26ba7bfdcd61a Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 14 May 2026 12:05:25 +0200 Subject: [PATCH] OF-3277: Introduce helper method to determine database 'limit' keyword The SQL LIMIT clause is not portable across the databases Openfire supports. SQL Server, for example, uses `SELECT TOP`. Up until now, code had to interrogate the type of database to find out what variant to be used. In this commit, this information is exposed as metadata. Small changes have been applid to the pubsub persistence implementation. It is expected that the Monitoring plugin (and others) benefit from this, too. --- .../database/DbConnectionManager.java | 80 +++++++++++++++++++ .../DefaultPubSubPersistenceProvider.java | 21 +++-- .../database/DbConnectionManagerTest.java | 75 +++++++++++++++++ 3 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 xmppserver/src/test/java/org/jivesoftware/database/DbConnectionManagerTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/database/DbConnectionManager.java b/xmppserver/src/main/java/org/jivesoftware/database/DbConnectionManager.java index d143f1769e..45f6c34db8 100644 --- a/xmppserver/src/main/java/org/jivesoftware/database/DbConnectionManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/database/DbConnectionManager.java @@ -1143,6 +1143,33 @@ public static String getIdentifierQuoteString() { return identifierQuoteString; } + /** + * Returns the keyword or keyword phrase used by the active database to limit result sets. + * + * Depending on the database, this can be {@link ResultSetLimitKeyword#TOP}, + * {@link ResultSetLimitKeyword#LIMIT}, or {@link ResultSetLimitKeyword#FETCH_FIRST}. + * + * @return the result-set limit keyword used by the current database. + * @see OF-3277 + */ + public static ResultSetLimitKeyword getResultSetLimitKeyword() + { + return databaseType.getResultSetLimitKeyword(); + } + + /** + * Indicates if the result-set limit keyword is used as a prefix before the selected columns. + * + * This is true for databases such as SQL Server that use {@code SELECT TOP (...) ...}. + * + * @return {@code true} if the keyword is prefix-style, otherwise {@code false}. + * @see OF-3277 + */ + public static boolean isResultSetLimitKeywordPrefix() + { + return databaseType.isResultSetLimitKeywordPrefix(); + } + public static String getTestSQL(String driver) { if (driver == null) { return "select 1"; @@ -1196,5 +1223,58 @@ public String escapeIdentifier(final String keyword) { return keyword; } } + + /** + * Returns the keyword or keyword phrase that should be used to limit result sets for this database type. + * + * @return the result-set limit keyword, or a sensible default when the database type is unknown. + * @see OF-3277 + */ + public ResultSetLimitKeyword getResultSetLimitKeyword() + { + return switch (this) { + case sqlserver -> ResultSetLimitKeyword.TOP; + case oracle, db2 -> ResultSetLimitKeyword.FETCH_FIRST; + default -> ResultSetLimitKeyword.LIMIT; + }; + } + + /** + * Indicates if the result-set limit keyword is used before the column list in a SELECT statement. + * + * @return {@code true} when the keyword is prefix-style (for example {@code TOP}), otherwise {@code false}. + * @see OF-3277 + */ + public boolean isResultSetLimitKeywordPrefix() + { + return getResultSetLimitKeyword().isPrefix(); + } + } + + /** + * Identifies how a database limits result sets. + * + * @see OF-3277 + */ + public enum ResultSetLimitKeyword + { + TOP(true), + FETCH_FIRST(false), + LIMIT(false); + + private final boolean prefix; + + ResultSetLimitKeyword(final boolean prefix) { + this.prefix = prefix; + } + + /** + * Indicates if the keyword is used before the selected columns in a {@code SELECT} statement. + * + * @return {@code true} for prefix-style keywords such as {@code TOP}, otherwise {@code false}. + */ + public boolean isPrefix() { + return prefix; + } } } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/DefaultPubSubPersistenceProvider.java b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/DefaultPubSubPersistenceProvider.java index 7ca1d05946..8461780658 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/DefaultPubSubPersistenceProvider.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/pubsub/DefaultPubSubPersistenceProvider.java @@ -1631,21 +1631,26 @@ else if (maxPublished != -1) { con = DbConnectionManager.getConnection(); // Get published items of the specified node - if (DbConnectionManager.getDatabaseType().equals(DbConnectionManager.DatabaseType.sqlserver)) { - pstmt = con.prepareStatement(LOAD_LAST_ITEMS_TOP); - } else if (DbConnectionManager.getDatabaseType().equals(DbConnectionManager.DatabaseType.oracle)) { - pstmt = con.prepareStatement(LOAD_LAST_ITEMS_FETCHFIRST); - } else { - pstmt = con.prepareStatement(LOAD_LAST_ITEMS_LIMIT); + switch (DbConnectionManager.getDatabaseType().getResultSetLimitKeyword()) { + case TOP: + pstmt = con.prepareStatement(LOAD_LAST_ITEMS_TOP); + break; + case FETCH_FIRST: + pstmt = con.prepareStatement(LOAD_LAST_ITEMS_FETCHFIRST); + break; + case LIMIT: // Intended fall-through + default: + pstmt = con.prepareStatement(LOAD_LAST_ITEMS_LIMIT); + break; } pstmt.setMaxRows(max); int paramIndex = 0; - if (DbConnectionManager.getDatabaseType().equals(DbConnectionManager.DatabaseType.sqlserver)){ + if (DbConnectionManager.getDatabaseType().isResultSetLimitKeywordPrefix()) { pstmt.setLong(++paramIndex, max); } pstmt.setString(++paramIndex, node.getUniqueIdentifier().getServiceIdentifier().getServiceId()); pstmt.setString(++paramIndex, encodeNodeID(node.getNodeID())); - if (!DbConnectionManager.getDatabaseType().equals(DbConnectionManager.DatabaseType.sqlserver)){ + if (!DbConnectionManager.getDatabaseType().isResultSetLimitKeywordPrefix()) { pstmt.setLong(++paramIndex, max); } rs = pstmt.executeQuery(); diff --git a/xmppserver/src/test/java/org/jivesoftware/database/DbConnectionManagerTest.java b/xmppserver/src/test/java/org/jivesoftware/database/DbConnectionManagerTest.java new file mode 100644 index 0000000000..f1cd2562d5 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/database/DbConnectionManagerTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jivesoftware.database; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class DbConnectionManagerTest { + + /** + * Verifies that SQL Server uses TOP as a prefix-style row limiting keyword. + */ + @Test + public void sqlServerUsesTopAsAPrefixKeyword() { + assertEquals(DbConnectionManager.ResultSetLimitKeyword.TOP, DbConnectionManager.DatabaseType.sqlserver.getResultSetLimitKeyword(), "SQL Server should map to the TOP limit keyword."); + assertTrue(DbConnectionManager.DatabaseType.sqlserver.isResultSetLimitKeywordPrefix(), "TOP should be marked as a prefix-style keyword."); + } + + /** + * Verifies that Oracle uses FETCH FIRST as a suffix-style row limiting keyword. + */ + @Test + public void oracleUsesFetchFirstAsASuffixKeyword() { + assertEquals(DbConnectionManager.ResultSetLimitKeyword.FETCH_FIRST, DbConnectionManager.DatabaseType.oracle.getResultSetLimitKeyword(), "Oracle should map to the FETCH_FIRST limit keyword."); + assertFalse(DbConnectionManager.DatabaseType.oracle.isResultSetLimitKeywordPrefix(), "FETCH_FIRST should be marked as a suffix-style keyword."); + } + + /** + * Verifies that DB2 uses FETCH FIRST as a suffix-style row limiting keyword. + */ + @Test + public void db2UsesFetchFirstAsASuffixKeyword() { + assertEquals(DbConnectionManager.ResultSetLimitKeyword.FETCH_FIRST, DbConnectionManager.DatabaseType.db2.getResultSetLimitKeyword(), "DB2 should map to the FETCH_FIRST limit keyword."); + assertFalse(DbConnectionManager.DatabaseType.db2.isResultSetLimitKeywordPrefix(), "DB2 should use a suffix-style limit keyword."); + } + + /** + * Verifies that common ANSI-style databases map to LIMIT. + */ + @Test + public void commonAnsiStyleDatabasesUseLimit() { + assertEquals(DbConnectionManager.ResultSetLimitKeyword.LIMIT, DbConnectionManager.DatabaseType.mysql.getResultSetLimitKeyword(), "MySQL should map to the LIMIT keyword."); + assertEquals(DbConnectionManager.ResultSetLimitKeyword.LIMIT, DbConnectionManager.DatabaseType.postgresql.getResultSetLimitKeyword(), "PostgreSQL should map to the LIMIT keyword."); + assertEquals(DbConnectionManager.ResultSetLimitKeyword.LIMIT, DbConnectionManager.DatabaseType.hsqldb.getResultSetLimitKeyword(), "HSQLDB should map to the LIMIT keyword."); + assertEquals(DbConnectionManager.ResultSetLimitKeyword.LIMIT, DbConnectionManager.DatabaseType.unknown.getResultSetLimitKeyword(), "Unknown database types should default to LIMIT."); + } + + /** + * Verifies prefix-position metadata for all supported row limiting keywords. + */ + @Test + public void enumKnowsWhetherItIsPrefixStyle() { + assertTrue(DbConnectionManager.ResultSetLimitKeyword.TOP.isPrefix(), "TOP should be treated as a prefix-style keyword."); + assertFalse(DbConnectionManager.ResultSetLimitKeyword.FETCH_FIRST.isPrefix(), "FETCH_FIRST should be treated as a suffix-style keyword."); + assertFalse(DbConnectionManager.ResultSetLimitKeyword.LIMIT.isPrefix(), "LIMIT should be treated as a suffix-style keyword."); + } +} + +