From ff5c0d0d619e6f2c5da6d6aebe5967d6124c9570 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 3 Feb 2022 10:20:31 -0600 Subject: [PATCH 1/9] 821-support-sort-null-handling - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 2c64717780..1539b78bae 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 0646c2846d..c55d9ccad0 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 11114a795e..f76dc28f46 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index a6eb48c891..f862f08876 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 2.4.0-SNAPSHOT + 2.4.0-821-support-sort-null-handling-SNAPSHOT From 4e87a374dbd48ab468237ff45105c929c0fc281b Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 3 Feb 2022 10:28:47 -0600 Subject: [PATCH 2/9] Add ORDER BY null handling option support for postgres and hsql. --- .../data/jdbc/core/convert/SqlGenerator.java | 2 +- ...JdbcAggregateTemplateIntegrationTests.java | 13 ++++ .../core/convert/SqlGeneratorUnitTests.java | 60 ++++++++++++++++++- .../core/dialect/AbstractDialect.java | 14 ++++- .../data/relational/core/dialect/Dialect.java | 4 ++ .../core/dialect/HsqlDbDialect.java | 5 ++ .../core/dialect/OrderByOptionsSupport.java | 9 +++ .../core/dialect/OrderByOptionsSupported.java | 30 ++++++++++ .../core/dialect/PostgresDialect.java | 5 ++ .../core/sql/render/OrderByClauseVisitor.java | 8 +-- .../core/sql/render/SelectRenderContext.java | 8 +++ .../PostgresDialectRenderingUnitTests.java | 57 ++++++++++++++++++ .../SqlServerDialectRenderingUnitTests.java | 17 ++++++ 13 files changed, 224 insertions(+), 8 deletions(-) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 80fa379a49..0f9731239e 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -714,7 +714,7 @@ private OrderByField orderToOrderByField(Sort.Order order) { SqlIdentifier columnName = this.entity.getRequiredPersistentProperty(order.getProperty()).getColumnName(); Column column = Column.create(columnName, this.getTable()); - return OrderByField.from(column, order.getDirection()); + return OrderByField.from(column, order.getDirection()).withNullHandling(order.getNullHandling()); } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 4153ac6b3b..3ea6d42e1c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -257,6 +257,19 @@ void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() { .containsExactly("Star"); } + @Test // GH-821 + void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullHandling() { + template.save(createLegoSet(null)); + template.save(createLegoSet("Star")); + template.save(createLegoSet("Frozen")); + + Iterable reloadedLegoSets = template.findAll(LegoSet.class, Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); + + assertThat(reloadedLegoSets) // + .extracting("name") // + .containsExactly("Frozen", "Star", null); + } + @Test // DATAJDBC-112 @EnabledOnFeature(SUPPORTS_QUOTED_IDS) void saveAndLoadManyEntitiesByIdWithReferencedEntity() { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 363d9c9d1d..a83e82c640 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -18,6 +18,7 @@ import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.SoftAssertions.*; +import static org.mockito.Mockito.*; import static org.springframework.data.relational.core.sql.SqlIdentifier.*; import java.util.Map; @@ -25,7 +26,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; - import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Version; @@ -37,8 +37,12 @@ import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.dialect.AbstractDialect; import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.relational.core.dialect.Dialect; +import org.springframework.data.relational.core.dialect.LimitClause; +import org.springframework.data.relational.core.dialect.LockClause; +import org.springframework.data.relational.core.dialect.OrderByOptionsSupport; import org.springframework.data.relational.core.dialect.PostgresDialect; import org.springframework.data.relational.core.dialect.SqlServerDialect; import org.springframework.data.relational.core.mapping.Column; @@ -48,6 +52,7 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.Aliased; +import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; @@ -245,6 +250,59 @@ void findAllSortedByMultipleFields() { "x_other ASC"); } + @Test + void findAllSortedWithNullHandling_resolvesDialectSupportedOrderByOptions() { + LimitClause limitClause = mock(LimitClause.class); + when(limitClause.getClausePosition()).thenReturn(LimitClause.Position.AFTER_ORDER_BY); + LockClause lockClause = mock(LockClause.class); + when(lockClause.getClausePosition()).thenReturn(LockClause.Position.AFTER_ORDER_BY); + OrderByOptionsSupport orderByOptionsSupport = mock(OrderByOptionsSupport.class); + Sort.Direction direction = Sort.Direction.ASC; + Sort.NullHandling nullHandling = Sort.NullHandling.NULLS_LAST; + String sortPart = "ASCENDING NULLS @ END"; + when(orderByOptionsSupport.resolve(direction, nullHandling)).thenReturn(sortPart); + SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, + new TestDialect(limitClause, lockClause, orderByOptionsSupport)); + + String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(direction, "name", nullHandling))); + + assertThat(sql).contains(String.format("ORDER BY dummy_entity.x_name %s", sortPart)); + } + + static class TestDialect extends AbstractDialect { + private final LimitClause limitClause; + private final LockClause lockClause; + private final OrderByOptionsSupport orderByOptionsSupport; + + public TestDialect(LimitClause limitClause, + LockClause lockClause, + OrderByOptionsSupport orderByOptionsSupport) { + this.limitClause = limitClause; + this.lockClause = lockClause; + this.orderByOptionsSupport = orderByOptionsSupport; + } + + @Override + public LimitClause limit() { + return this.limitClause; + } + + @Override + public LockClause lock() { + return this.lockClause; + } + + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return orderByOptionsSupport; + } + + @Override + public IdentifierProcessing getIdentifierProcessing() { + return IdentifierProcessing.NONE; + } + } + @Test // DATAJDBC-101 void findAllPagedByUnpaged() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java index 6563df66a3..4dcda4c630 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java @@ -18,10 +18,13 @@ import java.util.OptionalLong; import java.util.function.Function; +import org.springframework.data.domain.Sort; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.render.SelectRenderContext; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; /** * Base class for {@link Dialect} implementations. @@ -42,7 +45,7 @@ public SelectRenderContext getSelectContext() { Function afterFromTable = getAfterFromTable(); Function afterOrderBy = getAfterOrderBy(); - return new DialectSelectRenderContext(afterFromTable, afterOrderBy); + return new DialectSelectRenderContext(afterFromTable, afterOrderBy, orderByOptionsSupport()); } /** @@ -105,12 +108,14 @@ static class DialectSelectRenderContext implements SelectRenderContext { private final Function afterFromTable; private final Function afterOrderBy; + private final OrderByOptionsSupport orderByOptionsSupport; DialectSelectRenderContext(Function afterFromTable, - Function afterOrderBy) { + Function afterOrderBy, OrderByOptionsSupport orderByOptionsSupport) { this.afterFromTable = afterFromTable; this.afterOrderBy = afterOrderBy; + this.orderByOptionsSupport = orderByOptionsSupport; } /* @@ -130,6 +135,11 @@ static class DialectSelectRenderContext implements SelectRenderContext { public Function afterOrderBy(boolean hasOrderBy) { return afterOrderBy; } + + @Override + public String resolveOrderByOptions(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + return orderByOptionsSupport.resolve(direction, nullHandling); + } } /** diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index 5febb8c52f..5d88ba0d49 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -120,4 +120,8 @@ default Set> simpleTypes() { default InsertRenderContext getInsertRenderContext() { return InsertRenderContexts.DEFAULT; } + + default OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.DEFAULT; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index cf534de202..7621d3ccfc 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -37,6 +37,11 @@ public LockClause lock() { return AnsiDialect.LOCK_CLAUSE; } + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.NULL_HANDLING; + } + private static final LimitClause LIMIT_CLAUSE = new LimitClause() { @Override diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java new file mode 100644 index 0000000000..11bccff8a9 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java @@ -0,0 +1,9 @@ +package org.springframework.data.relational.core.dialect; + +import org.springframework.data.domain.Sort; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; + +public interface OrderByOptionsSupport { + String resolve(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling); +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java new file mode 100644 index 0000000000..1e09a8ec31 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java @@ -0,0 +1,30 @@ +package org.springframework.data.relational.core.dialect; + +import java.util.StringJoiner; + +import org.springframework.data.domain.Sort; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; + +public enum OrderByOptionsSupported implements OrderByOptionsSupport { + NULL_HANDLING(true), + DEFAULT(false); + + private final boolean supportNullHandling; + + OrderByOptionsSupported(boolean supportNullHandling) { + this.supportNullHandling = supportNullHandling; + } + + @Override + public String resolve(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + StringJoiner stringJoiner = new StringJoiner(" "); + if (direction != null) { + stringJoiner.add(direction.toString()); + } + if (supportNullHandling && !Sort.NullHandling.NATIVE.equals(nullHandling)) { + stringJoiner.add(nullHandling.toString().replace("_", " ")); + } + return stringJoiner.toString(); + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index 8782bbf3da..5148ca1bf1 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -230,6 +230,11 @@ public Set> simpleTypes() { return Collections.unmodifiableSet(simpleTypes); } + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.NULL_HANDLING; + } + /** * If the class is present on the class path, invoke the specified consumer {@code action} with the class object, * otherwise do nothing. diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java index a918231dd2..edfc62ebba 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java @@ -59,11 +59,11 @@ Delegation enterMatched(OrderByField segment) { @Override Delegation leaveMatched(OrderByField segment) { - OrderByField field = segment; - - if (field.getDirection() != null) { + String orderByOptions = context.getSelectRenderContext() + .resolveOrderByOptions(segment.getDirection(), segment.getNullHandling()); + if (!orderByOptions.isEmpty()) { builder.append(" ") // - .append(field.getDirection()); + .append(orderByOptions); } return Delegation.leave(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java index 74592f3915..9ed07246c6 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java @@ -18,8 +18,12 @@ import java.util.OptionalLong; import java.util.function.Function; +import org.springframework.data.domain.Sort; +import org.springframework.data.relational.core.dialect.OrderByOptionsSupported; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.Select; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; /** * Render context specifically for {@code SELECT} statements. This interface declares rendering hooks that are called @@ -86,4 +90,8 @@ public interface SelectRenderContext { return lockPrefix; }; } + + default String resolveOrderByOptions(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + return OrderByOptionsSupported.DEFAULT.resolve(direction, nullHandling); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index c2aca005e3..e49638013c 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -20,7 +20,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.data.domain.Sort; +import org.springframework.data.relational.core.sql.Column; import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.OrderByField; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.StatementBuilder; import org.springframework.data.relational.core.sql.Table; @@ -147,4 +150,58 @@ public void shouldRenderSelectWithLimitWithLockRead() { assertThat(sql).isEqualTo("SELECT foo.* FROM foo LIMIT 10 FOR SHARE OF foo"); } + + @Test // GH-821 + void shouldRenderSelectOrderByWithNoOptions() { + Table table = Table.create("foo"); + Select select = StatementBuilder.select(table.asterisk()) + .from(table) + .orderBy(OrderByField.from(Column.create("bar", table))) + .build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.bar"); + } + + @Test // GH-821 + void shouldRenderSelectOrderByWithDirection() { + Table table = Table.create("foo"); + Select select = StatementBuilder.select(table.asterisk()) + .from(table) + .orderBy(OrderByField.from(Column.create("bar", table), Sort.Direction.ASC)) + .build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.bar ASC"); + } + + @Test // GH-821 + void shouldRenderSelectOrderByWithNullHandling() { + Table table = Table.create("foo"); + Select select = StatementBuilder.select(table.asterisk()) + .from(table) + .orderBy(OrderByField.from(Column.create("bar", table)) + .withNullHandling(Sort.NullHandling.NULLS_FIRST)) + .build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.bar NULLS FIRST"); + } + + @Test // GH-821 + void shouldRenderSelectOrderByWithDirectionAndNullHandling() { + Table table = Table.create("foo"); + Select select = StatementBuilder.select(table.asterisk()) + .from(table) + .orderBy(OrderByField.from(Column.create("bar", table), Sort.Direction.DESC) + .withNullHandling(Sort.NullHandling.NULLS_FIRST)) + .build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.bar DESC NULLS FIRST"); + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java index 2c2f865c09..a6edbed48d 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java @@ -20,7 +20,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.data.domain.Sort; +import org.springframework.data.relational.core.sql.Column; import org.springframework.data.relational.core.sql.LockMode; +import org.springframework.data.relational.core.sql.OrderByField; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.StatementBuilder; import org.springframework.data.relational.core.sql.Table; @@ -192,4 +195,18 @@ public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockRead() { assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (HOLDLOCK, ROWLOCK) ORDER BY foo.column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY"); } + + @Test // GH-821 + void shouldRenderSelectOrderByIgnoringNullHandling() { + Table table = Table.create("foo"); + Select select = StatementBuilder.select(table.asterisk()) + .from(table) + .orderBy(OrderByField.from(Column.create("bar", table)) + .withNullHandling(Sort.NullHandling.NULLS_FIRST)) + .build(); + + String sql = SqlRenderer.create(factory.createRenderContext()).render(select); + + assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.bar"); + } } From bef6afea6f9bad621751df1926f5906c2757e5d9 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 3 Feb 2022 11:00:19 -0600 Subject: [PATCH 3/9] Add ORDER BY null handling option support for oracle, db2, and h2 dialects. --- .../data/relational/core/dialect/Db2Dialect.java | 5 +++++ .../data/relational/core/dialect/H2Dialect.java | 5 +++++ .../data/relational/core/dialect/OracleDialect.java | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index 82f527a148..f9b5ef5f11 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -118,4 +118,9 @@ public IdentifierProcessing getIdentifierProcessing() { public Collection getConverters() { return Collections.singletonList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE); } + + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.NULL_HANDLING; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index 7444edde27..5a19fcd818 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -158,4 +158,9 @@ public Set> simpleTypes() { throw new IllegalStateException(e); } } + + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.NULL_HANDLING; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java index 90f7d466e7..eeb389d901 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java @@ -56,6 +56,11 @@ public Collection getConverters() { return asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE, BooleanToIntegerConverter.INSTANCE); } + @Override + public OrderByOptionsSupport orderByOptionsSupport() { + return OrderByOptionsSupported.NULL_HANDLING; + } + @ReadingConverter enum NumberToBooleanConverter implements Converter { INSTANCE; From 57fd3982d2d659f81c704a0ea8b4201b2d0bcfd9 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 3 Feb 2022 11:51:35 -0600 Subject: [PATCH 4/9] Update null handling integration test to only run for supported stores. --- .../jdbc/core/JdbcAggregateTemplateIntegrationTests.java | 1 + .../data/jdbc/testing/TestDatabaseFeatures.java | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 3ea6d42e1c..555489680b 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -258,6 +258,7 @@ void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() { } @Test // GH-821 + @EnabledOnFeature({SUPPORTS_QUOTED_IDS, SUPPORTS_NULL_HANDLING}) void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullHandling() { template.save(createLegoSet(null)); template.save(createLegoSet("Star")); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java index 6e3e65a484..b9d6a4518c 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java @@ -83,6 +83,10 @@ private void supportsMultiDimensionalArrays() { assumeThat(database).isNotIn(Database.H2, Database.Hsql); } + private void supportsNullHandling() { + assumeThat(database).isNotIn(Database.MySql, Database.MariaDb, Database.SqlServer); + } + public void databaseIs(Database database) { assumeThat(this.database).isEqualTo(database); } @@ -115,6 +119,7 @@ public enum Feature { SUPPORTS_ARRAYS(TestDatabaseFeatures::supportsArrays), // SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES(TestDatabaseFeatures::supportsGeneratedIdsInReferencedEntities), // SUPPORTS_NANOSECOND_PRECISION(TestDatabaseFeatures::supportsNanosecondPrecision), // + SUPPORTS_NULL_HANDLING(TestDatabaseFeatures::supportsNullHandling), IS_POSTGRES(f -> f.databaseIs(Database.PostgreSql)), // IS_HSQL(f -> f.databaseIs(Database.Hsql)); From eb78b1dda05ee88671c67769fa95e9eba57af8ec Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Thu, 3 Feb 2022 13:30:43 -0600 Subject: [PATCH 5/9] Update license headers, class headers, and documentation. --- .../data/jdbc/core/convert/SqlGenerator.java | 3 ++- .../core/convert/SqlGeneratorUnitTests.java | 5 +++-- .../jdbc/testing/TestDatabaseFeatures.java | 3 ++- .../core/dialect/AbstractDialect.java | 3 ++- .../relational/core/dialect/Db2Dialect.java | 3 ++- .../data/relational/core/dialect/Dialect.java | 8 +++++++- .../relational/core/dialect/H2Dialect.java | 3 ++- .../core/dialect/HsqlDbDialect.java | 3 ++- .../core/dialect/OracleDialect.java | 3 ++- .../core/dialect/OrderByOptionsSupport.java | 20 +++++++++++++++++++ .../core/dialect/OrderByOptionsSupported.java | 20 +++++++++++++++++++ .../core/dialect/PostgresDialect.java | 3 ++- .../core/sql/render/OrderByClauseVisitor.java | 3 ++- .../core/sql/render/SelectRenderContext.java | 12 +++++++++-- .../PostgresDialectRenderingUnitTests.java | 3 ++- .../SqlServerDialectRenderingUnitTests.java | 3 ++- 16 files changed, 82 insertions(+), 16 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 0f9731239e..a95c9b1933 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2021 the original author or authors. + * Copyright 2017-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,6 +51,7 @@ * @author Milan Milanov * @author Myeonghyeon Lee * @author Mikhail Polivakha + * @author Chirag Tailor */ class SqlGenerator { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index a83e82c640..b12d650629 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2021 the original author or authors. + * Copyright 2017-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -69,6 +69,7 @@ * @author Milan Milanov * @author Myeonghyeon Lee * @author Mikhail Polivakha + * @author Chirag Tailor */ class SqlGeneratorUnitTests { @@ -250,7 +251,7 @@ void findAllSortedByMultipleFields() { "x_other ASC"); } - @Test + @Test // GH-821 void findAllSortedWithNullHandling_resolvesDialectSupportedOrderByOptions() { LimitClause limitClause = mock(LimitClause.class); when(limitClause.getClausePosition()).thenReturn(LimitClause.Position.AFTER_ORDER_BY); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java index b9d6a4518c..d252745034 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 the original author or authors. + * Copyright 2020-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ * presence or absence of features in tests. * * @author Jens Schauder + * @author Chirag Tailor */ public class TestDatabaseFeatures { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java index 4dcda4c630..d7cadb1c8b 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ * * @author Mark Paluch * @author Myeonghyeon Lee + * @author Chirag Tailor * @since 1.1 */ public abstract class AbstractDialect implements Dialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index f9b5ef5f11..c82f186047 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 the original author or authors. + * Copyright 2020-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ * An SQL dialect for DB2. * * @author Jens Schauder + * @author Chirag Tailor * @since 2.0 */ public class Db2Dialect extends AbstractDialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index 5d88ba0d49..948b7cb8d7 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,6 +33,7 @@ * @author Myeonghyeon Lee * @author Christoph Strobl * @author Mikhail Polivakha + * @author Chirag Tailor * @since 1.1 */ public interface Dialect { @@ -121,6 +122,11 @@ default InsertRenderContext getInsertRenderContext() { return InsertRenderContexts.DEFAULT; } + /** + * Return the {@link OrderByOptionsSupport} used by this dialect. + * + * @return the {@link OrderByOptionsSupport} used by this dialect. + */ default OrderByOptionsSupport orderByOptionsSupport() { return OrderByOptionsSupported.DEFAULT; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index 5a19fcd818..ac4c4236bd 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,7 @@ * @author Mark Paluch * @author Myeonghyeon Lee * @author Christph Strobl + * @author Chirag Tailor * @since 2.0 */ public class H2Dialect extends AbstractDialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index 7621d3ccfc..2f0dffca8d 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ * * @author Jens Schauder * @author Myeonghyeon Lee + * @author Chirag Tailor */ public class HsqlDbDialect extends AbstractDialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java index eeb389d901..cd891d54cc 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ * An SQL dialect for Oracle. * * @author Jens Schauder + * @author Chirag Tailor * @since 2.1 */ public class OracleDialect extends AnsiDialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java index 11bccff8a9..be338d81a9 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java @@ -1,9 +1,29 @@ +/* + * Copyright 2022 the original author or authors. + * + * 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 + * + * https://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.springframework.data.relational.core.dialect; import org.springframework.data.domain.Sort; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; +/** + * The {@code ORDER BY} options supported by a specific Dialect. + * + * @author Chirag Tailor + */ public interface OrderByOptionsSupport { String resolve(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java index 1e09a8ec31..df59d1ac48 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java @@ -1,3 +1,18 @@ +/* + * Copyright 2022 the original author or authors. + * + * 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 + * + * https://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.springframework.data.relational.core.dialect; import java.util.StringJoiner; @@ -6,6 +21,11 @@ import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; +/** + * This enum represents the different sets of {@code ORDER BY} options that are supported by different {@link Dialect}s. + * + * @author Chirag Tailor + */ public enum OrderByOptionsSupported implements OrderByOptionsSupport { NULL_HANDLING(true), DEFAULT(false); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index 5148ca1bf1..257bb225c5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,6 +39,7 @@ * @author Myeonghyeon Lee * @author Jens Schauder * @author Nikita Konev + * @author Chirag Tailor * @since 1.1 */ public class PostgresDialect extends AbstractDialect { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java index edfc62ebba..cbdbfb0577 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Chirag Tailor * @since 1.1 */ class OrderByClauseVisitor extends TypedSubtreeVisitor implements PartRenderer { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java index 9ed07246c6..037ff44083 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,12 +27,13 @@ /** * Render context specifically for {@code SELECT} statements. This interface declares rendering hooks that are called - * before/after a specific {@code SELECT} clause part. The rendering content is appended directly after/before an + * before/after/during a specific {@code SELECT} clause part. The rendering content is appended directly after/before/during an * element without further whitespace processing. Hooks are responsible for adding required surrounding whitespaces. * * @author Mark Paluch * @author Myeonghyeon Lee * @author Jens Schauder + * @author Chirag Tailor * @since 1.1 */ public interface SelectRenderContext { @@ -91,6 +92,13 @@ public interface SelectRenderContext { }; } + /** + * Customization hook: Rendition of the options for {@code ORDER BY}. + * + * @param direction the {@link Sort.Direction} for the {@code ORDER BY} clause. May be {@literal null}. + * @param nullHandling the {@link Sort.NullHandling} for the {@code ORDER BY} clause. Must not be {@literal null}. + * @return render the complete {@link String} options for the {@code ORDER BY} clause. + */ default String resolveOrderByOptions(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { return OrderByOptionsSupported.DEFAULT.resolve(direction, nullHandling); } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index e49638013c..de87acf15b 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ * @author Mark Paluch * @author Jens Schauder * @author Myeonghyeon Lee + * @author Chirag Tailor */ public class PostgresDialectRenderingUnitTests { diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java index a6edbed48d..eae958266c 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2021 the original author or authors. + * Copyright 2019-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ * @author Mark Paluch * @author Jens Schauder * @author Myeonghyeon Lee + * @author Chirag Tailor */ public class SqlServerDialectRenderingUnitTests { From b6a300ec2421b3dc3c1c9d6ead52800bb2dfe051 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 4 Feb 2022 08:48:21 -0600 Subject: [PATCH 6/9] Address code style issues. --- ...JdbcAggregateTemplateIntegrationTests.java | 1 + .../core/convert/SqlGeneratorUnitTests.java | 65 ++++--------------- .../core/dialect/AbstractDialect.java | 3 +- .../core/dialect/OrderByOptionsSupport.java | 3 +- .../core/dialect/OrderByOptionsSupported.java | 4 +- .../core/sql/render/SelectRenderContext.java | 2 +- .../PostgresDialectRenderingUnitTests.java | 4 ++ .../SqlServerDialectRenderingUnitTests.java | 1 + 8 files changed, 24 insertions(+), 59 deletions(-) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 555489680b..fa79aec3bd 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -260,6 +260,7 @@ void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() { @Test // GH-821 @EnabledOnFeature({SUPPORTS_QUOTED_IDS, SUPPORTS_NULL_HANDLING}) void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullHandling() { + template.save(createLegoSet(null)); template.save(createLegoSet("Star")); template.save(createLegoSet("Frozen")); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index b12d650629..7ea24dfebe 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -18,7 +18,6 @@ import static java.util.Collections.*; import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.SoftAssertions.*; -import static org.mockito.Mockito.*; import static org.springframework.data.relational.core.sql.SqlIdentifier.*; import java.util.Map; @@ -37,12 +36,8 @@ import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; import org.springframework.data.mapping.PersistentPropertyPath; -import org.springframework.data.relational.core.dialect.AbstractDialect; import org.springframework.data.relational.core.dialect.AnsiDialect; import org.springframework.data.relational.core.dialect.Dialect; -import org.springframework.data.relational.core.dialect.LimitClause; -import org.springframework.data.relational.core.dialect.LockClause; -import org.springframework.data.relational.core.dialect.OrderByOptionsSupport; import org.springframework.data.relational.core.dialect.PostgresDialect; import org.springframework.data.relational.core.dialect.SqlServerDialect; import org.springframework.data.relational.core.mapping.Column; @@ -52,7 +47,6 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.Aliased; -import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; @@ -252,56 +246,23 @@ void findAllSortedByMultipleFields() { } @Test // GH-821 - void findAllSortedWithNullHandling_resolvesDialectSupportedOrderByOptions() { - LimitClause limitClause = mock(LimitClause.class); - when(limitClause.getClausePosition()).thenReturn(LimitClause.Position.AFTER_ORDER_BY); - LockClause lockClause = mock(LockClause.class); - when(lockClause.getClausePosition()).thenReturn(LockClause.Position.AFTER_ORDER_BY); - OrderByOptionsSupport orderByOptionsSupport = mock(OrderByOptionsSupport.class); - Sort.Direction direction = Sort.Direction.ASC; - Sort.NullHandling nullHandling = Sort.NullHandling.NULLS_LAST; - String sortPart = "ASCENDING NULLS @ END"; - when(orderByOptionsSupport.resolve(direction, nullHandling)).thenReturn(sortPart); - SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, - new TestDialect(limitClause, lockClause, orderByOptionsSupport)); - - String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(direction, "name", nullHandling))); - - assertThat(sql).contains(String.format("ORDER BY dummy_entity.x_name %s", sortPart)); - } - - static class TestDialect extends AbstractDialect { - private final LimitClause limitClause; - private final LockClause lockClause; - private final OrderByOptionsSupport orderByOptionsSupport; - - public TestDialect(LimitClause limitClause, - LockClause lockClause, - OrderByOptionsSupport orderByOptionsSupport) { - this.limitClause = limitClause; - this.lockClause = lockClause; - this.orderByOptionsSupport = orderByOptionsSupport; - } + void findAllSortedWithNullHandling_resolvesNullHandlingWhenDialectSupportsIt() { - @Override - public LimitClause limit() { - return this.limitClause; - } + SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, PostgresDialect.INSTANCE); - @Override - public LockClause lock() { - return this.lockClause; - } + String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); - @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return orderByOptionsSupport; - } + assertThat(sql).contains("ORDER BY \"dummy_entity\".\"x_name\" ASC NULLS LAST"); + } - @Override - public IdentifierProcessing getIdentifierProcessing() { - return IdentifierProcessing.NONE; - } + @Test // GH-821 + void findAllSortedWithNullHandling_ignoresNullHandlingWhenDialectDoesNotSupportIt() { + + SqlGenerator sqlGenerator = createSqlGenerator(DummyEntity.class, SqlServerDialect.INSTANCE); + + String sql = sqlGenerator.getFindAll(Sort.by(new Sort.Order(Sort.Direction.ASC, "name", Sort.NullHandling.NULLS_LAST))); + + assertThat(sql).endsWith("ORDER BY dummy_entity.x_name ASC"); } @Test // DATAJDBC-101 diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java index d7cadb1c8b..75782d411e 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java @@ -23,7 +23,6 @@ import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.render.SelectRenderContext; -import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; /** @@ -138,7 +137,7 @@ static class DialectSelectRenderContext implements SelectRenderContext { } @Override - public String resolveOrderByOptions(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + public String resolveOrderByOptions(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { return orderByOptionsSupport.resolve(direction, nullHandling); } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java index be338d81a9..a941c75262 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java @@ -16,7 +16,6 @@ package org.springframework.data.relational.core.dialect; import org.springframework.data.domain.Sort; -import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; /** @@ -25,5 +24,5 @@ * @author Chirag Tailor */ public interface OrderByOptionsSupport { - String resolve(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling); + String resolve(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java index df59d1ac48..069476ee29 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java @@ -18,7 +18,6 @@ import java.util.StringJoiner; import org.springframework.data.domain.Sort; -import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; /** @@ -37,7 +36,8 @@ public enum OrderByOptionsSupported implements OrderByOptionsSupport { } @Override - public String resolve(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + public String resolve(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { + StringJoiner stringJoiner = new StringJoiner(" "); if (direction != null) { stringJoiner.add(direction.toString()); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java index 037ff44083..b3b85d2819 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java @@ -99,7 +99,7 @@ public interface SelectRenderContext { * @param nullHandling the {@link Sort.NullHandling} for the {@code ORDER BY} clause. Must not be {@literal null}. * @return render the complete {@link String} options for the {@code ORDER BY} clause. */ - default String resolveOrderByOptions(@Nullable Sort.Direction direction, @NonNull Sort.NullHandling nullHandling) { + default String resolveOrderByOptions(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { return OrderByOptionsSupported.DEFAULT.resolve(direction, nullHandling); } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java index de87acf15b..ddf09cdcfc 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/PostgresDialectRenderingUnitTests.java @@ -154,6 +154,7 @@ public void shouldRenderSelectWithLimitWithLockRead() { @Test // GH-821 void shouldRenderSelectOrderByWithNoOptions() { + Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()) .from(table) @@ -167,6 +168,7 @@ void shouldRenderSelectOrderByWithNoOptions() { @Test // GH-821 void shouldRenderSelectOrderByWithDirection() { + Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()) .from(table) @@ -180,6 +182,7 @@ void shouldRenderSelectOrderByWithDirection() { @Test // GH-821 void shouldRenderSelectOrderByWithNullHandling() { + Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()) .from(table) @@ -194,6 +197,7 @@ void shouldRenderSelectOrderByWithNullHandling() { @Test // GH-821 void shouldRenderSelectOrderByWithDirectionAndNullHandling() { + Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()) .from(table) diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java index eae958266c..403faa241f 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java @@ -199,6 +199,7 @@ public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockRead() { @Test // GH-821 void shouldRenderSelectOrderByIgnoringNullHandling() { + Table table = Table.create("foo"); Select select = StatementBuilder.select(table.asterisk()) .from(table) From 1e2389e07379f0606e6e51d4b75029c928c3d1f1 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 4 Feb 2022 10:37:32 -0600 Subject: [PATCH 7/9] Refactor Dialect null handling to only have a single responsibility. Move ORDER BY direction evaluation back into OrderByClauseVisitor. --- .../core/dialect/AbstractDialect.java | 13 ++-- .../relational/core/dialect/Db2Dialect.java | 4 +- .../data/relational/core/dialect/Dialect.java | 8 +-- .../relational/core/dialect/H2Dialect.java | 4 +- .../core/dialect/HsqlDbDialect.java | 4 +- .../core/dialect/OracleDialect.java | 5 +- .../core/dialect/OrderByNullHandling.java | 64 +++++++++++++++++++ .../core/dialect/OrderByOptionsSupport.java | 28 -------- .../core/dialect/OrderByOptionsSupported.java | 50 --------------- .../core/dialect/PostgresDialect.java | 4 +- .../core/sql/render/OrderByClauseVisitor.java | 12 ++-- .../core/sql/render/SelectRenderContext.java | 17 ++--- 12 files changed, 99 insertions(+), 114 deletions(-) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java index 75782d411e..7043a9f734 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/AbstractDialect.java @@ -23,7 +23,6 @@ import org.springframework.data.relational.core.sql.LockOptions; import org.springframework.data.relational.core.sql.Select; import org.springframework.data.relational.core.sql.render.SelectRenderContext; -import org.springframework.lang.Nullable; /** * Base class for {@link Dialect} implementations. @@ -45,7 +44,7 @@ public SelectRenderContext getSelectContext() { Function afterFromTable = getAfterFromTable(); Function afterOrderBy = getAfterOrderBy(); - return new DialectSelectRenderContext(afterFromTable, afterOrderBy, orderByOptionsSupport()); + return new DialectSelectRenderContext(afterFromTable, afterOrderBy, orderByNullHandling()); } /** @@ -108,14 +107,14 @@ static class DialectSelectRenderContext implements SelectRenderContext { private final Function afterFromTable; private final Function afterOrderBy; - private final OrderByOptionsSupport orderByOptionsSupport; + private final OrderByNullHandling orderByNullHandling; DialectSelectRenderContext(Function afterFromTable, - Function afterOrderBy, OrderByOptionsSupport orderByOptionsSupport) { + Function afterOrderBy, OrderByNullHandling orderByNullHandling) { this.afterFromTable = afterFromTable; this.afterOrderBy = afterOrderBy; - this.orderByOptionsSupport = orderByOptionsSupport; + this.orderByNullHandling = orderByNullHandling; } /* @@ -137,8 +136,8 @@ static class DialectSelectRenderContext implements SelectRenderContext { } @Override - public String resolveOrderByOptions(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { - return orderByOptionsSupport.resolve(direction, nullHandling); + public String evaluateOrderByNullHandling(Sort.NullHandling nullHandling) { + return orderByNullHandling.evaluate(nullHandling); } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index c82f186047..254404bc7b 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -121,7 +121,7 @@ public Collection getConverters() { } @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.NULL_HANDLING; + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.SQL_STANDARD; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index 948b7cb8d7..a8ef3094f4 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -123,11 +123,11 @@ default InsertRenderContext getInsertRenderContext() { } /** - * Return the {@link OrderByOptionsSupport} used by this dialect. + * Return the {@link OrderByNullHandling} used by this dialect. * - * @return the {@link OrderByOptionsSupport} used by this dialect. + * @return the {@link OrderByNullHandling} used by this dialect. */ - default OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.DEFAULT; + default OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.NONE; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index ac4c4236bd..62b757515a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -161,7 +161,7 @@ public Set> simpleTypes() { } @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.NULL_HANDLING; + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.SQL_STANDARD; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index 2f0dffca8d..b1afc3a535 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -39,8 +39,8 @@ public LockClause lock() { } @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.NULL_HANDLING; + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.SQL_STANDARD; } private static final LimitClause LIMIT_CLAUSE = new LimitClause() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java index cd891d54cc..04f20ce35a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java @@ -20,7 +20,6 @@ import org.springframework.data.convert.WritingConverter; import java.util.Collection; -import java.util.Collections; import static java.util.Arrays.*; @@ -58,8 +57,8 @@ public Collection getConverters() { } @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.NULL_HANDLING; + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.SQL_STANDARD; } @ReadingConverter diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java new file mode 100644 index 0000000000..0bb6dc429b --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java @@ -0,0 +1,64 @@ +/* + * Copyright 2022 the original author or authors. + * + * 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 + * + * https://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.springframework.data.relational.core.dialect; + +import org.springframework.data.domain.Sort; + +/** + * Represents how the {@link Sort.NullHandling} option of an {@code ORDER BY} sort expression is to be evaluated. + * + * @author Chirag Tailor + */ +public interface OrderByNullHandling { + /** + * An {@link OrderByNullHandling} that can be used for databases conforming to the SQL standard which uses + * {@code NULLS FIRST} and {@code NULLS LAST} in {@code ORDER BY} sort expressions to make null values appear before + * or after non-null values in the result set. + */ + OrderByNullHandling SQL_STANDARD = new SqlStandardOrderByNullHandling(); + + /** + * An {@link OrderByNullHandling} that can be used for databases that do not support the SQL standard usage of + * {@code NULLS FIRST} and {@code NULLS LAST} in {@code ORDER BY} sort expressions to control where null values appear + * respective to non-null values in the result set. + */ + OrderByNullHandling NONE = nullHandling -> ""; + + /** + * Converts a {@link Sort.NullHandling} option to the appropriate SQL text to be included an {@code ORDER BY} sort + * expression. + */ + String evaluate(Sort.NullHandling nullHandling); + + /** + * An {@link OrderByNullHandling} implementation for databases conforming to the SQL standard which uses + * {@code NULLS FIRST} and {@code NULLS LAST} in {@code ORDER BY} sort expressions to make null values appear before + * or after non-null values in the result set. + * + * @author Chirag Tailor + */ + class SqlStandardOrderByNullHandling implements OrderByNullHandling { + + @Override + public String evaluate(Sort.NullHandling nullHandling) { + + if (Sort.NullHandling.NATIVE.equals(nullHandling)) { + return ""; + } + return nullHandling.toString().replace("_", " "); + } + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java deleted file mode 100644 index a941c75262..0000000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupport.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - * - * 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 - * - * https://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.springframework.data.relational.core.dialect; - -import org.springframework.data.domain.Sort; -import org.springframework.lang.Nullable; - -/** - * The {@code ORDER BY} options supported by a specific Dialect. - * - * @author Chirag Tailor - */ -public interface OrderByOptionsSupport { - String resolve(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling); -} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java deleted file mode 100644 index 069476ee29..0000000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByOptionsSupported.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - * - * 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 - * - * https://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.springframework.data.relational.core.dialect; - -import java.util.StringJoiner; - -import org.springframework.data.domain.Sort; -import org.springframework.lang.Nullable; - -/** - * This enum represents the different sets of {@code ORDER BY} options that are supported by different {@link Dialect}s. - * - * @author Chirag Tailor - */ -public enum OrderByOptionsSupported implements OrderByOptionsSupport { - NULL_HANDLING(true), - DEFAULT(false); - - private final boolean supportNullHandling; - - OrderByOptionsSupported(boolean supportNullHandling) { - this.supportNullHandling = supportNullHandling; - } - - @Override - public String resolve(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { - - StringJoiner stringJoiner = new StringJoiner(" "); - if (direction != null) { - stringJoiner.add(direction.toString()); - } - if (supportNullHandling && !Sort.NullHandling.NATIVE.equals(nullHandling)) { - stringJoiner.add(nullHandling.toString().replace("_", " ")); - } - return stringJoiner.toString(); - } -} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index 257bb225c5..6b8277d070 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -232,8 +232,8 @@ public Set> simpleTypes() { } @Override - public OrderByOptionsSupport orderByOptionsSupport() { - return OrderByOptionsSupported.NULL_HANDLING; + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.SQL_STANDARD; } /** diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java index cbdbfb0577..949a234a60 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java @@ -60,11 +60,15 @@ Delegation enterMatched(OrderByField segment) { @Override Delegation leaveMatched(OrderByField segment) { - String orderByOptions = context.getSelectRenderContext() - .resolveOrderByOptions(segment.getDirection(), segment.getNullHandling()); - if (!orderByOptions.isEmpty()) { + if (segment.getDirection() != null) { builder.append(" ") // - .append(orderByOptions); + .append(segment.getDirection()); + } + + String nullHandling = context.getSelectRenderContext().evaluateOrderByNullHandling(segment.getNullHandling()); + if (!nullHandling.isEmpty()) { + builder.append(" ") // + .append(nullHandling); } return Delegation.leave(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java index b3b85d2819..12bbaa0d5e 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectRenderContext.java @@ -19,15 +19,13 @@ import java.util.function.Function; import org.springframework.data.domain.Sort; -import org.springframework.data.relational.core.dialect.OrderByOptionsSupported; +import org.springframework.data.relational.core.dialect.OrderByNullHandling; import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.Select; -import org.springframework.lang.NonNull; -import org.springframework.lang.Nullable; /** * Render context specifically for {@code SELECT} statements. This interface declares rendering hooks that are called - * before/after/during a specific {@code SELECT} clause part. The rendering content is appended directly after/before/during an + * before/after/during a specific {@code SELECT} clause part. The rendering content is appended directly after/before an * element without further whitespace processing. Hooks are responsible for adding required surrounding whitespaces. * * @author Mark Paluch @@ -93,13 +91,12 @@ public interface SelectRenderContext { } /** - * Customization hook: Rendition of the options for {@code ORDER BY}. + * Customization hook: Rendition of the null handling option for an {@code ORDER BY} sort expression. * - * @param direction the {@link Sort.Direction} for the {@code ORDER BY} clause. May be {@literal null}. - * @param nullHandling the {@link Sort.NullHandling} for the {@code ORDER BY} clause. Must not be {@literal null}. - * @return render the complete {@link String} options for the {@code ORDER BY} clause. + * @param nullHandling the {@link Sort.NullHandling} for the {@code ORDER BY} sort expression. Must not be {@literal null}. + * @return render {@link String} SQL text to be included in an {@code ORDER BY} sort expression. */ - default String resolveOrderByOptions(@Nullable Sort.Direction direction, Sort.NullHandling nullHandling) { - return OrderByOptionsSupported.DEFAULT.resolve(direction, nullHandling); + default String evaluateOrderByNullHandling(Sort.NullHandling nullHandling) { + return OrderByNullHandling.NONE.evaluate(nullHandling); } } From bf306b67cdb9039e0dfebe24fb97c97bd05e8d6a Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 4 Feb 2022 11:31:12 -0600 Subject: [PATCH 8/9] Decouple sql standard null handling evaluation from enum names. This makes the implementation more explicit and resilient to possible upstream code changes. --- .../core/dialect/OrderByNullHandling.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java index 0bb6dc429b..4dcadc8eff 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OrderByNullHandling.java @@ -52,13 +52,20 @@ public interface OrderByNullHandling { */ class SqlStandardOrderByNullHandling implements OrderByNullHandling { + private static final String NULLS_FIRST = "NULLS FIRST"; + private static final String NULLS_LAST = "NULLS LAST"; + private static final String UNSPECIFIED = ""; + @Override public String evaluate(Sort.NullHandling nullHandling) { - if (Sort.NullHandling.NATIVE.equals(nullHandling)) { - return ""; + switch (nullHandling) { + case NULLS_FIRST: return NULLS_FIRST; + case NULLS_LAST: return NULLS_LAST; + case NATIVE: return UNSPECIFIED; + default: + throw new UnsupportedOperationException("Sort.NullHandling " + nullHandling + " not supported"); } - return nullHandling.toString().replace("_", " "); } } } From 9e512c781eac6264b3f54142e0bd30b082657c90 Mon Sep 17 00:00:00 2001 From: Chirag Tailor Date: Fri, 4 Feb 2022 12:06:57 -0600 Subject: [PATCH 9/9] Change Dialect default OrderByNullHandling to SQL_STANDARD as more dbs conform than do not. --- .../data/relational/core/dialect/Db2Dialect.java | 5 ----- .../data/relational/core/dialect/Dialect.java | 2 +- .../data/relational/core/dialect/H2Dialect.java | 5 ----- .../data/relational/core/dialect/HsqlDbDialect.java | 5 ----- .../data/relational/core/dialect/MySqlDialect.java | 5 +++++ .../data/relational/core/dialect/OracleDialect.java | 5 ----- .../data/relational/core/dialect/PostgresDialect.java | 5 ----- .../data/relational/core/dialect/SqlServerDialect.java | 5 +++++ 8 files changed, 11 insertions(+), 26 deletions(-) diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java index 254404bc7b..f3860c1fe6 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Db2Dialect.java @@ -119,9 +119,4 @@ public IdentifierProcessing getIdentifierProcessing() { public Collection getConverters() { return Collections.singletonList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE); } - - @Override - public OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.SQL_STANDARD; - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java index a8ef3094f4..eee10f0b6c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/Dialect.java @@ -128,6 +128,6 @@ default InsertRenderContext getInsertRenderContext() { * @return the {@link OrderByNullHandling} used by this dialect. */ default OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.NONE; + return OrderByNullHandling.SQL_STANDARD; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java index 62b757515a..ee6d370785 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/H2Dialect.java @@ -159,9 +159,4 @@ public Set> simpleTypes() { throw new IllegalStateException(e); } } - - @Override - public OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.SQL_STANDARD; - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java index b1afc3a535..96016eb283 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/HsqlDbDialect.java @@ -38,11 +38,6 @@ public LockClause lock() { return AnsiDialect.LOCK_CLAUSE; } - @Override - public OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.SQL_STANDARD; - } - private static final LimitClause LIMIT_CLAUSE = new LimitClause() { @Override 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 6032582186..a1a9919293 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 @@ -169,4 +169,9 @@ public IdentifierProcessing getIdentifierProcessing() { public Collection getConverters() { return Collections.singletonList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE); } + + @Override + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.NONE; + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java index 04f20ce35a..40fc7142ff 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java @@ -56,11 +56,6 @@ public Collection getConverters() { return asList(TimestampAtUtcToOffsetDateTimeConverter.INSTANCE, NumberToBooleanConverter.INSTANCE, BooleanToIntegerConverter.INSTANCE); } - @Override - public OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.SQL_STANDARD; - } - @ReadingConverter enum NumberToBooleanConverter implements Converter { INSTANCE; diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java index 6b8277d070..815fe53aeb 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java @@ -231,11 +231,6 @@ public Set> simpleTypes() { return Collections.unmodifiableSet(simpleTypes); } - @Override - public OrderByNullHandling orderByNullHandling() { - return OrderByNullHandling.SQL_STANDARD; - } - /** * If the class is present on the class path, invoke the specified consumer {@code action} with the class object, * otherwise do nothing. diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java index 11ae0d3cf3..7f393e05e1 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlServerDialect.java @@ -156,4 +156,9 @@ public IdentifierProcessing getIdentifierProcessing() { public InsertRenderContext getInsertRenderContext() { return InsertRenderContexts.MS_SQL_SERVER; } + + @Override + public OrderByNullHandling orderByNullHandling() { + return OrderByNullHandling.NONE; + } }