Skip to content

Use fully qualified names in ORDER BY clause. #1080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Use fully qualified names in ORDER BY clause. #1080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a5aa78
968-fully-qualified-order-by - Prepare branch
schauder Nov 4, 2021
349f70d
Use fully qualified names in ORDER BY clause.
schauder Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data Relational Parent</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-jdbc-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jdbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-jdbc</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>

<name>Spring Data JDBC</name>
<description>Spring Data module for JDBC repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>
</parent>

<properties>
Expand Down
11 changes: 6 additions & 5 deletions ...-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public void findAllSortedBySingleField() {
"FROM dummy_entity ", //
"LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1", //
"LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id", //
"ORDER BY x_name ASC");
"ORDER BY dummy_entity.x_name ASC");
}

@Test // DATAJDBC-101
Expand All @@ -238,7 +238,7 @@ public void findAllSortedByMultipleFields() {
"FROM dummy_entity ", //
"LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1", //
"LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id", //
"ORDER BY x_name DESC", //
"ORDER BY dummy_entity.x_name DESC", //
"x_other ASC");
}

Expand Down Expand Up @@ -286,7 +286,7 @@ public void findAllPagedAndSorted() {
"FROM dummy_entity ", //
"LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1", //
"LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id", //
"ORDER BY x_name ASC", //
"ORDER BY dummy_entity.x_name ASC", //
"OFFSET 30", //
"LIMIT 10");
}
Expand Down Expand Up @@ -371,7 +371,8 @@ public void findAllByPropertyWithKeyOrdered() {
+ "FROM dummy_entity " //
+ "LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1 " //
+ "LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id " //
+ "WHERE dummy_entity.backref = :backref " + "ORDER BY key-column");
+ "WHERE dummy_entity.backref = :backref " //
+ "ORDER BY dummy_entity.key-column");
}

@Test // DATAJDBC-219
Expand Down Expand Up @@ -493,7 +494,7 @@ public void readOnlyPropertyIncludedIntoQuery_when_generateFindAllByPropertySql(
+ "entity_with_read_only_property.key-column AS key-column " //
+ "FROM entity_with_read_only_property " //
+ "WHERE entity_with_read_only_property.backref = :backref " //
+ "ORDER BY key-column" //
+ "ORDER BY entity_with_read_only_property.key-column" //
);
}

Expand Down
4 changes: 2 additions & 2 deletions .../test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ public void createsQueryToFindAllEntitiesByIntegerAttributeWithDescendingOrderin
ParametrizedQuery query = jdbcQuery.createQuery(accessor, returnedType);

assertThat(query.getQuery())
.isEqualTo(BASE_SELECT + " WHERE " + TABLE + ".\"AGE\" = :age ORDER BY \"LAST_NAME\" DESC");
.isEqualTo(BASE_SELECT + " WHERE " + TABLE + ".\"AGE\" = :age ORDER BY \"users\".\"LAST_NAME\" DESC");
}

@Test // DATAJDBC-318
Expand All @@ -456,7 +456,7 @@ public void createsQueryToFindAllEntitiesByIntegerAttributeWithAscendingOrdering
ParametrizedQuery query = jdbcQuery.createQuery(accessor, returnedType);

assertThat(query.getQuery())
.isEqualTo(BASE_SELECT + " WHERE " + TABLE + ".\"AGE\" = :age ORDER BY \"LAST_NAME\" ASC");
.isEqualTo(BASE_SELECT + " WHERE " + TABLE + ".\"AGE\" = :age ORDER BY \"users\".\"LAST_NAME\" ASC");
}

@Test // DATAJDBC-318
Expand Down
4 changes: 2 additions & 2 deletions spring-data-relational/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-relational</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>

<name>Spring Data Relational</name>
<description>Spring Data Relational support</description>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>2.3.0-SNAPSHOT</version>
<version>2.3.0-968-fully-qualified-order-by-SNAPSHOT</version>
</parent>

<properties>
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Delegation leaveMatched(OrderByField segment) {
Delegation leaveNested(Visitable segment) {

if (segment instanceof Column) {
builder.append(NameRenderer.reference(context, (Column) segment));
builder.append(NameRenderer.fullyQualifiedReference(context, (Column) segment));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this consider aliased column names as we want to order by a specific thing that is being selected (SELECT foo.bar as fb, baz.bar as foo.bar … ORDER BY fb)?

}

return super.leaveNested(segment);
Expand Down
6 changes: 3 additions & 3 deletions .../org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void shouldRenderSelectWithLimitOffsetAndOrderBy() {

String sql = SqlRenderer.create(factory.createRenderContext()).render(select);

assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
assertThat(sql).isEqualTo("SELECT foo.* FROM foo ORDER BY foo.column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
}

@Test // DATAJDBC-498
Expand Down Expand Up @@ -177,7 +177,7 @@ public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockWrite() {

String sql = SqlRenderer.create(factory.createRenderContext()).render(select);

assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (UPDLOCK, ROWLOCK) ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (UPDLOCK, ROWLOCK) ORDER BY foo.column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
}

@Test // DATAJDBC-498
Expand All @@ -190,6 +190,6 @@ public void shouldRenderSelectWithLimitOffsetAndOrderByWithLockRead() {

String sql = SqlRenderer.create(factory.createRenderContext()).render(select);

assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (HOLDLOCK, ROWLOCK) ORDER BY column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
assertThat(sql).isEqualTo("SELECT foo.* FROM foo WITH (HOLDLOCK, ROWLOCK) ORDER BY foo.column_1 OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
}
}
10 changes: 0 additions & 10 deletions .../test/java/org/springframework/data/relational/core/sql/render/NameRendererUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,4 @@ void fullyQualifiedReference() {

assertThat(rendered).isEqualTo("tab_alias.col_alias");
}

@Test // GH-1003
void fullyQualifiedUnaliasedReference() {

Column column = Column.aliased("col", Table.aliased("table", "tab_alias"), "col_alias");

CharSequence rendered = NameRenderer.fullyQualifiedUnaliasedReference(context, column);

assertThat(rendered).isEqualTo("tab_alias.col");
}
}
38 changes: 34 additions & 4 deletions ...va/org/springframework/data/relational/core/sql/render/OrderByClauseVisitorUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
public class OrderByClauseVisitorUnitTests {

@Test // DATAJDBC-309
public void shouldRenderOrderByName() {
public void shouldRenderOrderByAlias() {

Table employee = SQL.table("employee").as("emp");
Column column = employee.column("name").as("emp_name");
Expand All @@ -42,9 +42,8 @@ public void shouldRenderOrderByName() {
OrderByClauseVisitor visitor = new OrderByClauseVisitor(new SimpleRenderContext(NamingStrategies.asIs()));
select.visit(visitor);

assertThat(visitor.getRenderedPart().toString()).isEqualTo("emp_name ASC");
assertThat(visitor.getRenderedPart().toString()).isEqualTo("emp.emp_name ASC");
}

@Test // DATAJDBC-309
public void shouldApplyNamingStrategy() {

Expand All @@ -56,6 +55,37 @@ public void shouldApplyNamingStrategy() {
OrderByClauseVisitor visitor = new OrderByClauseVisitor(new SimpleRenderContext(NamingStrategies.toUpper()));
select.visit(visitor);

assertThat(visitor.getRenderedPart().toString()).isEqualTo("EMP_NAME ASC");
assertThat(visitor.getRenderedPart().toString()).isEqualTo("EMP.EMP_NAME ASC");
}

@Test // GH-968
public void shouldRenderOrderByFullyQualifiedName() {

Table employee = SQL.table("employee");
Column column = employee.column("name");

Select select = Select.builder().select(column).from(employee).orderBy(OrderByField.from(column).asc()).build();

OrderByClauseVisitor visitor = new OrderByClauseVisitor(new SimpleRenderContext(NamingStrategies.asIs()));
select.visit(visitor);

assertThat(visitor.getRenderedPart().toString()).isEqualTo("employee.name ASC");
}

@Test // GH-968
public void shouldRenderOrderByFullyQualifiedNameWithTableAlias() {

Table employee = SQL.table("employee").as("emp");
Column column = employee.column("name");

Select select = Select.builder().select(column).from(employee).orderBy(OrderByField.from(column).asc()).build();

OrderByClauseVisitor visitor = new OrderByClauseVisitor(new SimpleRenderContext(NamingStrategies.asIs()));
select.visit(visitor);

assertThat(visitor.getRenderedPart().toString()).isEqualTo("emp.name ASC");
}



}
24 changes: 23 additions & 1 deletion ...est/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ void shouldRenderOrderByName() {
Select select = Select.builder().select(column).from(employee).orderBy(OrderByField.from(column).asc()).build();

assertThat(SqlRenderer.toString(select))
.isEqualTo("SELECT emp.name AS emp_name FROM employee emp ORDER BY emp_name ASC");
.isEqualTo("SELECT emp.name AS emp_name FROM employee emp ORDER BY emp.emp_name ASC");
}

@Test // DATAJDBC-309
Expand Down Expand Up @@ -467,4 +467,26 @@ void shouldRenderCast() {
final String rendered = SqlRenderer.toString(select);
assertThat(rendered).isEqualTo("SELECT CAST(User.name AS VARCHAR2) FROM User");
}

@Test // GH-968
void rendersFullyQualifiedNamesInOrderBy() {

Table tableA = SQL.table("tableA");
Column tableAName = tableA.column("name");
Column tableAId = tableA.column("id");

Table tableB = SQL.table("tableB");
Column tableBId = tableB.column("id");
Column tableBName = tableB.column("name");

Select select = StatementBuilder.select(Expressions.asterisk()) //
.from(tableA) //
.join(tableB).on(tableAId.isEqualTo(tableBId)) //
.orderBy(tableAName, tableBName) //
.build();

final String rendered = SqlRenderer.toString(select);
assertThat(rendered)
.isEqualTo("SELECT * FROM tableA JOIN tableB ON tableA.id = tableB.id ORDER BY tableA.name, tableB.name");
}
}