DATAJDBC-491 - Correctly combine schema- and table name

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-relational-parent</artifactId>
-	<version>2.0.0.BUILD-SNAPSHOT</version>
+	<version>2.0.0.DATAJDBC-491-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data Relational Parent</name>

spring-data-jdbc-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>2.0.0.BUILD-SNAPSHOT</version>
+		<version>2.0.0.DATAJDBC-491-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-jdbc/pom.xml

@@ -6,7 +6,7 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-jdbc</artifactId>
-	<version>2.0.0.BUILD-SNAPSHOT</version>
+	<version>2.0.0.DATAJDBC-491-SNAPSHOT</version>
 
 	<name>Spring Data JDBC</name>
 	<description>Spring Data module for JDBC repositories.</description>
@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>2.0.0.BUILD-SNAPSHOT</version>
+		<version>2.0.0.DATAJDBC-491-SNAPSHOT</version>
 	</parent>
 
 	<properties>

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

@@ -83,16 +83,16 @@ public void findOneWithOverriddenFixedTableName() {
 		SoftAssertions softAssertions = new SoftAssertions();
 		softAssertions.assertThat(sql) //
 				.isEqualTo(
-						"SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", "
-								+ "\"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_NAME\" AS \"FIXEDCUSTOMPROPERTYPREFIX_NAME\", "
+						"SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", "
+								+ "\"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_NAME\" AS \"FIXEDCUSTOMPROPERTYPREFIX_NAME\", "
 								+ "\"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" AS \"REF_FIXEDCUSTOMPROPERTYPREFIX_L1ID\", "
 								+ "\"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_CONTENT\" AS \"REF_FIXEDCUSTOMPROPERTYPREFIX_CONTENT\", "
 								+ "\"ref_further\".\"FIXEDCUSTOMPROPERTYPREFIX_L2ID\" AS \"REF_FURTHER_FIXEDCUSTOMPROPERTYPREFIX_L2ID\", "
 								+ "\"ref_further\".\"FIXEDCUSTOMPROPERTYPREFIX_SOMETHING\" AS \"REF_FURTHER_FIXEDCUSTOMPROPERTYPREFIX_SOMETHING\" "
-								+ "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" "
-								+ "LEFT OUTER JOIN \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" AS \"ref\" ON \"ref\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" = \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" L"
-								+ "EFT OUTER JOIN \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" AS \"ref_further\" ON \"ref_further\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" = \"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-								+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id");
+								+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" "
+								+ "LEFT OUTER JOIN \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" AS \"ref\" ON \"ref\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" = \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" L"
+								+ "EFT OUTER JOIN \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" AS \"ref_further\" ON \"ref_further\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" = \"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
+								+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id");
 		softAssertions.assertAll();
 	}
 
@@ -122,8 +122,8 @@ public void cascadingDeleteFirstLevel() {
 
 		String sql = sqlGenerator.createDeleteByPath(getPath("ref"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId");
+		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+				+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId");
 	}
 
 	@Test // DATAJDBC-107
@@ -133,11 +133,12 @@ public void cascadingDeleteAllSecondLevel() {
 
 		String sql = sqlGenerator.createDeleteByPath(getPath("ref.further"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
-				+ "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-				+ "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)");
+		assertThat(sql)
+				.isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
+						+ "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
+						+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)");
 	}
 
 	@Test // DATAJDBC-107
@@ -147,7 +148,7 @@ public void deleteAll() {
 
 		String sql = sqlGenerator.createDeleteAllSql(null);
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\"");
+		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\"");
 	}
 
 	@Test // DATAJDBC-107
@@ -157,8 +158,8 @@ public void cascadingDeleteAllFirstLevel() {
 
 		String sql = sqlGenerator.createDeleteAllSql(getPath("ref"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL");
+		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+				+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL");
 	}
 
 	@Test // DATAJDBC-107
@@ -168,11 +169,12 @@ public void cascadingDeleteSecondLevel() {
 
 		String sql = sqlGenerator.createDeleteAllSql(getPath("ref.further"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
-				+ "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-				+ "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)");
+		assertThat(sql)
+				.isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
+						+ "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
+						+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)");
 	}
 
 	@Test // DATAJDBC-113
@@ -183,7 +185,7 @@ public void deleteByList() {
 		String sql = sqlGenerator.getDeleteByList();
 
 		assertThat(sql).isEqualTo(
-				"DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" IN (:ids)");
+				"DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" IN (:ids)");
 	}
 
 	private PersistentPropertyPath<RelationalPersistentProperty> getPath(String path) {

spring-data-relational/pom.xml

@@ -6,15 +6,15 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-relational</artifactId>
-	<version>2.0.0.BUILD-SNAPSHOT</version>
+	<version>2.0.0.DATAJDBC-491-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.0.0.BUILD-SNAPSHOT</version>
+		<version>2.0.0.DATAJDBC-491-SNAPSHOT</version>
 	</parent>
 
 	<properties>

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java

@@ -66,6 +66,7 @@ public String getKeyColumn(RelationalPersistentProperty property) {
 	 * @see org.springframework.data.relational.core.mapping.NamingStrategy#getQualifiedTableName(java.lang.Class)
 	 */
 	@Override
+	@Deprecated
 	public String getQualifiedTableName(Class<?> type) {
 		return qualifiedTableNames.computeIfAbsent(type, delegate::getQualifiedTableName);
 	}

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/NamingStrategy.java

@@ -72,6 +72,13 @@ default String getColumnName(RelationalPersistentProperty property) {
 		return ParsingUtils.reconcatenateCamelCase(property.getName(), "_");
 	}
 
+	/**
+	 * @param type
+	 * @return
+	 * @deprecated since 2.0. The method returns a concatenated schema with table name which conflicts with escaping. Use
+	 *             rather {@link #getTableName(Class)} and {@link #getSchema()} independently
+	 */
+	@Deprecated
 	default String getQualifiedTableName(Class<?> type) {
 		return this.getSchema() + (this.getSchema().equals("") ? "" : ".") + this.getTableName(type);
 	}

spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java

@@ -78,7 +78,14 @@ public void setForceQuote(boolean forceQuote) {
 	 */
 	@Override
 	public SqlIdentifier getTableName() {
-		return tableName.get().orElseGet(() -> createDerivedSqlIdentifier(namingStrategy.getQualifiedTableName(getType())));
+		return tableName.get().orElseGet(() -> {
+
+			String schema = namingStrategy.getSchema();
+			SqlIdentifier tableName = createDerivedSqlIdentifier(namingStrategy.getTableName(getType()));
+
+			return StringUtils.hasText(schema) ? SqlIdentifier.from(createDerivedSqlIdentifier(schema), tableName)
+					: tableName;
+		});
 	}
 
 	/*

spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImplUnitTests.java

@@ -19,14 +19,18 @@
 import static org.springframework.data.relational.core.sql.SqlIdentifier.*;
 
 import org.junit.Test;
+
 import org.springframework.data.annotation.Id;
+import org.springframework.data.relational.core.sql.IdentifierProcessing;
+import org.springframework.data.relational.core.sql.SqlIdentifier;
 
 /**
  * Unit tests for {@link RelationalPersistentEntityImpl}.
  *
  * @author Oliver Gierke
  * @author Kazuki Shimizu
  * @author Bastian Wilhelm
+ * @author Mark Paluch
  */
 public class RelationalPersistentEntityImplUnitTests {
 
@@ -56,6 +60,18 @@ public void emptyTableAnnotationFallsBackToNamingStrategy() {
 		assertThat(entity.getTableName()).isEqualTo(quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION"));
 	}
 
+	@Test // DATAJDBC-491
+	public void namingStrategyWithSchemaReturnsCompositeTableName() {
+
+		mappingContext = new RelationalMappingContext(NamingStrategyWithSchema.INSTANCE);
+		RelationalPersistentEntity<?> entity = mappingContext.getPersistentEntity(DummyEntityWithEmptyAnnotation.class);
+
+		assertThat(entity.getTableName())
+				.isEqualTo(SqlIdentifier.from(quoted("MY_SCHEMA"), quoted("DUMMY_ENTITY_WITH_EMPTY_ANNOTATION")));
+		assertThat(entity.getTableName().toSql(IdentifierProcessing.ANSI))
+				.isEqualTo("\"MY_SCHEMA\".\"DUMMY_ENTITY_WITH_EMPTY_ANNOTATION\"");
+	}
+
 	@Table("dummy_sub_entity")
 	static class DummySubEntity {
 		@Id @Column("renamedId") Long id;
@@ -65,4 +81,13 @@ static class DummySubEntity {
 	static class DummyEntityWithEmptyAnnotation {
 		@Id @Column() Long id;
 	}
+
+	enum NamingStrategyWithSchema implements NamingStrategy {
+		INSTANCE;
+
+		@Override
+		public String getSchema() {
+			return "my_schema";
+		}
+	}
 }