diff --git a/src/adql/translator/SQLServerTranslator.java b/src/adql/translator/SQLServerTranslator.java index 57078170f75693bb0bb8c128c59f770991a1a759..57b9cd4040c46a96bb15e1aa7b61e0a4a404abc4 100644 --- a/src/adql/translator/SQLServerTranslator.java +++ b/src/adql/translator/SQLServerTranslator.java @@ -16,7 +16,7 @@ package adql.translator; * You should have received a copy of the GNU Lesser General Public License * along with ADQLLibrary. If not, see <http://www.gnu.org/licenses/>. * - * Copyright 2017-2019 - Astronomisches Rechen Institut (ARI), + * Copyright 2017-2020 - Astronomisches Rechen Institut (ARI), * UDS/Centre de DonnĂ©es astronomiques de Strasbourg (CDS) */ @@ -67,6 +67,8 @@ import adql.query.operand.function.geometry.RegionFunction; * translate. * </p> * + * TODO Translation of Set operations, TOP/LIMIT, OFFSET, ORDER BY + * * TODO See how case sensitivity is supported by MS SQL Server and modify this translator accordingly. * * TODO Extend this class for each MS SQL Server extension supporting geometry and particularly @@ -89,7 +91,7 @@ import adql.query.operand.function.geometry.RegionFunction; * </i></p> * * @author Grégory Mantelet (ARI;CDS) - * @version 2.0 (08/2019) + * @version 2.0 (11/2020) * @since 1.4 * * @see SQLServer_ADQLQueryFactory @@ -208,14 +210,23 @@ public class SQLServerTranslator extends JDBCTranslator { * LIMIT and OFFSET handling. * * <p><i><b>Implementation note:</b> - * LIMIT is replaced by FETCH NEXT instead of TOP because of the addition - * of OFFSET support in ADQL-2.1 grammar. With MS-SQLServer, TOP can not be - * used with OFFSET...it must be OFFSET...LIMIT.... + * If an OFFSET is specified, TOP can no longer be used to specify a limit + * in SQL. In such case, TOP is replaced by FETCH NEXT right after the + * OFFSET instruction. Besides, SQLServer requires an ORDER BY clause in + * order to use OFFSET. If none is given in the ADQL query, the default + * <code>ORDER BY 1 ASC</code> (i.e. sort on the first column) is applied. * </i></p> */ @Override public String translate(ADQLQuery query) throws TranslationException { - StringBuffer sql = new StringBuffer(translate(query.getSelect())); + StringBuffer sql = new StringBuffer(); + + // Start with the SELECT clause: + /* NOTE: If a limit is specified, TOP should be used if no OFFSET is + * used, otherwise the limit must be specified right after + * OFFSET. */ + final boolean withOffset = (query.getOffset() != null && query.getOffset().getValue() > 0); + sql.append(translate(query.getSelect(), !withOffset)); sql.append("\nFROM ").append(translate(query.getFrom())); @@ -231,26 +242,45 @@ public class SQLServerTranslator extends JDBCTranslator { if (!query.getOrderBy().isEmpty()) sql.append('\n').append(translate(query.getOrderBy())); - if (query.getSelect().hasLimit()) { - if (query.getOffset() != null) - sql.append('\n').append("OFFSET ").append(query.getOffset().getValue()).append(" ROWS"); - else - sql.append('\n').append("OFFSET 0 ROWS"); - sql.append(" FETCH NEXT ").append(query.getSelect().getLimit()).append(" ROWS ONLY"); - } else if (query.getOffset() != null) { - sql.append('\n').append("OFFSET ").append(query.getOffset().getValue()).append(" ROWS"); + // Deal with OFFSET: + if (withOffset) { + + // An ORDER BY is required by OFFSET ; so, ensure there is one: + if (query.getOrderBy().isEmpty()) + sql.append("\nORDER BY 1 ASC"); // default: order on the 1st col + + // Append the OFFSET: + sql.append("\nOFFSET ").append(query.getOffset().getValue()).append(" ROWS"); + + // With OFFSET, the TOP/LIMIT must be expressed differently: + if (query.hasLimit()) + sql.append(" FETCH NEXT ").append(query.getLimit()).append(" ROWS ONLY"); } return sql.toString(); } - @Override - public String translate(ClauseSelect clause) throws TranslationException { + /** + * This version of {@link #translate(ClauseSelect)} lets translate the given + * SELECT clause with or without the TOP instruction in case a limit is + * specified in ADQL. + * + * @param clause The SELECT clause to translate. + * @param topAllowed If <code>true</code> and a TOP limit is specified, + * it will be translated as a TOP in SQL (exactly as + * in ADQL). + * If <code>false</code> and a TOP limit is specified, + * it will never appear in the SQL translation. <i>(in + * such case, {@link #translate(ADQLQuery)} will take + * care of this limit)</i> + * + * @since 2.0 */ + protected String translate(final ClauseSelect clause, final boolean topAllowed) throws TranslationException { String sql = null; for(int i = 0; i < clause.size(); i++) { if (i == 0) - sql = clause.getName() + (clause.distinctColumns() ? " DISTINCT" : ""); + sql = clause.getName() + (clause.distinctColumns() ? " DISTINCT" : "") + (topAllowed && clause.hasLimit() ? " TOP " + clause.getLimit() : ""); else sql += " " + clause.getSeparator(i); @@ -260,6 +290,21 @@ public class SQLServerTranslator extends JDBCTranslator { return sql; } + /** + * Translate the given SELECT clause into an SQL compatible with + * MS-SQLServer. + * + * <p><i><b>Note:</b> + * The TOP limit, if any, will always be present in the SQL translation. + * </i></p> + * + * @see #translate(ClauseSelect, boolean) + */ + @Override + public String translate(ClauseSelect clause) throws TranslationException { + return translate(clause, true); + } + @Override public String translate(Comparison comp) throws TranslationException { switch(comp.getOperator()) { diff --git a/test/adql/translator/TestSQLServerTranslator.java b/test/adql/translator/TestSQLServerTranslator.java index 89d34a11e00764416354e3c9939c7b1e88de92ce..f03a878bf572a2b0f89385876df58f7b40bc6817 100644 --- a/test/adql/translator/TestSQLServerTranslator.java +++ b/test/adql/translator/TestSQLServerTranslator.java @@ -52,17 +52,21 @@ public class TestSQLServerTranslator { try { - // CASE: Only OFFSET - assertEquals("SELECT *\nFROM foo\nOFFSET 10 ROWS", tr.translate(parser.parseQuery("Select * From foo OffSet 10"))); + // CASE: Only OFFSET = 0 => No OFFSET in SQL + assertEquals("SELECT *\nFROM foo", tr.translate(parser.parseQuery("Select * From foo OffSet 0"))); + + // CASE: Only TOP (or with OFFSET=0) + assertEquals("SELECT TOP 5 *\nFROM foo", tr.translate(parser.parseQuery("Select Top 5 * From foo"))); + assertEquals("SELECT TOP 5 *\nFROM foo", tr.translate(parser.parseQuery("Select Top 5 * From foo Offset 0"))); - // CASE: Only OFFSET = 0 - assertEquals("SELECT *\nFROM foo\nOFFSET 0 ROWS", tr.translate(parser.parseQuery("Select * From foo OffSet 0"))); + // CASE: Only OFFSET + assertEquals("SELECT *\nFROM foo\nORDER BY 1 ASC\nOFFSET 10 ROWS", tr.translate(parser.parseQuery("Select * From foo OffSet 10"))); - // CASE: TOP + OFFSET - assertEquals("SELECT *\nFROM foo\nOFFSET 10 ROWS FETCH NEXT 5 ROWS ONLY", tr.translate(parser.parseQuery("Select Top 5 * From foo OffSet 10"))); + // CASE: TOP + OFFSET but no ORDER BY + assertEquals("SELECT *\nFROM foo\nORDER BY 1 ASC\nOFFSET 10 ROWS FETCH NEXT 5 ROWS ONLY", tr.translate(parser.parseQuery("Select Top 5 * From foo OffSet 10"))); - // CASE: TOP + ORDER BY + OFFSET - assertEquals("SELECT *\nFROM foo\nORDER BY id ASC\nOFFSET 10 ROWS FETCH NEXT 5 ROWS ONLY", tr.translate(parser.parseQuery("Select Top 5 * From foo Order By id Asc OffSet 10"))); + // CASE: TOP + OFFSET + ORDER BY + assertEquals("SELECT *\nFROM foo\nORDER BY id DESC\nOFFSET 10 ROWS FETCH NEXT 5 ROWS ONLY", tr.translate(parser.parseQuery("Select Top 5 * From foo Order By id Desc OffSet 10"))); } catch(ParseException pe) { pe.printStackTrace(System.err);