Use fully qualified names in ORDER BY clause.

Closes #968
Original pull request: #1080.

Author: schauder

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java

@@ -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
@@ -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");
 	}
 
@@ -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");
 	}
@@ -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
@@ -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" //
 		);
 	}
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/PartTreeJdbcQueryUnitTests.java

@@ -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
@@ -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

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitor.java

@@ -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));
 		}
 
 		return super.leaveNested(segment);

spring-data-relational/src/test/java/org/springframework/data/relational/core/dialect/SqlServerDialectRenderingUnitTests.java

@@ -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
@@ -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
@@ -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");
 	}
 }

spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/NameRendererUnitTests.java

@@ -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");
-	}
 }

spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/OrderByClauseVisitorUnitTests.java

@@ -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");
@@ -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() {
 
@@ -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");
+	}
+
+
+
 }

spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java

@@ -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
@@ -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");
+	}
 }