From f499017269bf892ed920135512bb223dab389a3f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 9 Apr 2019 14:36:58 +0200 Subject: [PATCH] DATAJDBC-357 - Polishing. Implemented workaround for MySql not supporting offset without Limit. Using `SELECT 1` as dummy order by since it is documented to be optimized away. Renamed tests to match the project standard. See also: - https://stackoverflow.com/a/44106422 - https://stackoverflow.com/a/271650 Original pull request: #125. --- .../data/relational/core/dialect/LimitClause.java | 15 +++++++++------ .../relational/core/dialect/MySqlDialect.java | 7 +++++-- .../dialect/SqlServerSelectRenderContext.java | 2 +- ...s.java => MySqlDialectRenderingUnitTests.java} | 8 +++++--- .../core/dialect/MySqlDialectUnitTests.java | 3 ++- ...ava => PostgresDialectRenderingUnitTests.java} | 3 ++- ...va => SqlServerDialectRenderingUnitTests.java} | 9 +++++---- 7 files changed, 29 insertions(+), 18 deletions(-) rename spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/{MySqlDialectRenderingTests.java => MySqlDialectRenderingUnitTests.java} (90%) rename spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/{PostgresDialectRenderingTests.java => PostgresDialectRenderingUnitTests.java} (97%) rename spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/{SqlServerDialectRenderingTests.java => SqlServerDialectRenderingUnitTests.java} (86%) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LimitClause.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LimitClause.java index 70842d94..ca71b7a1 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LimitClause.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/LimitClause.java @@ -19,6 +19,7 @@ package org.springframework.data.relational.core.dialect; * A clause representing Dialect-specific {@code LIMIT}. * * @author Mark Paluch + * @author Jens Schauder * @since 1.1 */ public interface LimitClause { @@ -26,7 +27,7 @@ public interface LimitClause { /** * Returns the {@code LIMIT} clause to limit results. * - * @param limit the actual limit to use. + * @param limit the maximum number of lines returned when the resulting SQL snippet is used. * @return rendered limit clause. * @see #getLimitOffset(long, long) */ @@ -35,19 +36,21 @@ public interface LimitClause { /** * Returns the {@code OFFSET} clause to consume rows at a given offset. * - * @param limit the actual limit to use. - * @return rendered limit clause. + * @param offset the numbers of rows that get skipped when the resulting SQL snippet is used. + * @return rendered offset clause. * @see #getLimitOffset(long, long) */ - String getOffset(long limit); + String getOffset(long offset); /** * Returns a combined {@code LIMIT/OFFSET} clause that limits results and starts consumption at the given * {@code offset}. * - * @param limit the actual limit to use. - * @param offset the offset to start from. + * @param limit the maximum number of lines returned when the resulting SQL snippet is used. + * @param offset the numbers of rows that get skipped when the resulting SQL snippet is used. * @return rendered limit clause. + * @see #getLimit(long) + * @see #getOffset(long) */ String getLimitOffset(long limit, long offset); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java index 93efa5ee..09c36454 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/MySqlDialect.java @@ -19,6 +19,7 @@ package org.springframework.data.relational.core.dialect; * An SQL dialect for MySQL. * * @author Mark Paluch + * @author Jens Schauder * @since 1.1 */ public class MySqlDialect extends AbstractDialect { @@ -45,7 +46,9 @@ public class MySqlDialect extends AbstractDialect { */ @Override public String getOffset(long offset) { - throw new UnsupportedOperationException("MySQL does not support OFFSET without LIMIT"); + // Ugly but the official workaround for offset without limit + // see: https://stackoverflow.com/a/271650 + return String.format("LIMIT %d, 18446744073709551615", offset); } /* @@ -56,7 +59,7 @@ public class MySqlDialect extends AbstractDialect { public String getLimitOffset(long limit, long offset) { // LIMIT {[offset,] row_count} - return String.format("LIMIT %d, %d", offset, limit); + return String.format("LIMIT %s, %s", offset, limit); } /* diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java index 6f10669e..d044429e 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerSelectRenderContext.java @@ -33,7 +33,7 @@ public class SqlServerSelectRenderContext implements SelectRenderContext { private static final String SYNTHETIC_ORDER_BY_FIELD = "__relational_row_number__"; - private static final String SYNTHETIC_SELECT_LIST = ", ROW_NUMBER() over (ORDER BY CURRENT_TIMESTAMP) AS " + private static final String SYNTHETIC_SELECT_LIST = ", ROW_NUMBER() over (ORDER BY (SELECT 1)) AS " + SYNTHETIC_ORDER_BY_FIELD; private final Function afterOrderBy; diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java similarity index 90% rename from spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingTests.java rename to spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java index 49f0b62b..35f1cca0 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectRenderingUnitTests.java @@ -30,8 +30,9 @@ import org.springframework.data.relational.core.sql.render.SqlRenderer; * Tests for {@link MySqlDialect}-specific rendering. * * @author Mark Paluch + * @author Jens Schauder */ -public class MySqlDialectRenderingTests { +public class MySqlDialectRenderingUnitTests { private final RenderContextFactory factory = new RenderContextFactory(MySqlDialect.INSTANCE); @@ -57,8 +58,9 @@ public class MySqlDialectRenderingTests { Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()).from(table).offset(10).build(); - assertThatThrownBy(() -> SqlRenderer.create(factory.createRenderContext()).render(select)) - .isInstanceOf(UnsupportedOperationException.class); + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10, 18446744073709551615"); } @Test // DATAJDBC-278 diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java index 392bda48..16979e44 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/MySqlDialectUnitTests.java @@ -23,6 +23,7 @@ import org.junit.Test; * Unit tests for {@link MySqlDialect}. * * @author Mark Paluch + * @author Jens Schauder */ public class MySqlDialectUnitTests { @@ -48,7 +49,7 @@ public class MySqlDialectUnitTests { LimitClause limit = MySqlDialect.INSTANCE.limit(); - assertThatThrownBy(() -> limit.getOffset(10)).isInstanceOf(UnsupportedOperationException.class); + assertThat(limit.getOffset(10)).isEqualTo("LIMIT 10, 18446744073709551615"); } @Test // DATAJDBC-278 diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java similarity index 97% rename from spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingTests.java rename to spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index 23e37cdb..d6013f19 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -30,8 +30,9 @@ import org.springframework.data.relational.core.sql.render.SqlRenderer; * Tests for {@link PostgresDialect}-specific rendering. * * @author Mark Paluch + * @author Jens Schauder */ -public class PostgresDialectRenderingTests { +public class PostgresDialectRenderingUnitTests { private final RenderContextFactory factory = new RenderContextFactory(PostgresDialect.INSTANCE); diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java similarity index 86% rename from spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingTests.java rename to spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java index 35f6ff42..477b84d8 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java @@ -30,8 +30,9 @@ import org.springframework.data.relational.core.sql.render.SqlRenderer; * Tests for {@link SqlServerDialect}-specific rendering. * * @author Mark Paluch + * @author Jens Schauder */ -public class SqlServerDialectRenderingTests { +public class SqlServerDialectRenderingUnitTests { private final RenderContextFactory factory = new RenderContextFactory(SqlServerDialect.INSTANCE); @@ -73,7 +74,7 @@ public class SqlServerDialectRenderingTests { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); assertThat(sql).isEqualTo( - "SELECT foo.*, ROW_NUMBER() over (ORDER BY CURRENT_TIMESTAMP) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY"); + "SELECT foo.*, ROW_NUMBER() over (ORDER BY (SELECT 1)) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY"); } @Test // DATAJDBC-278 @@ -85,7 +86,7 @@ public class SqlServerDialectRenderingTests { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); assertThat(sql).isEqualTo( - "SELECT foo.*, ROW_NUMBER() over (ORDER BY CURRENT_TIMESTAMP) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 10 ROWS"); + "SELECT foo.*, ROW_NUMBER() over (ORDER BY (SELECT 1)) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 10 ROWS"); } @Test // DATAJDBC-278 @@ -97,7 +98,7 @@ public class SqlServerDialectRenderingTests { String sql = SqlRenderer.create(factory.createRenderContext()).render(select); assertThat(sql).isEqualTo( - "SELECT foo.*, ROW_NUMBER() over (ORDER BY CURRENT_TIMESTAMP) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); + "SELECT foo.*, ROW_NUMBER() over (ORDER BY (SELECT 1)) AS __relational_row_number__ FROM foo ORDER BY __relational_row_number__ OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); } @Test // DATAJDBC-278