DATAJDBC-491 - Correctly combine schema- and table name.

We no correctly combine schema and table name using separate SqlIdentifier objects instead of leaving the schema name as part of the table name. Previously, the concatenated name failed to resolve with quoted identifiers.

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

@@ -83,16 +83,16 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 		SoftAssertions softAssertions = new SoftAssertions();
 		softAssertions.assertThat(sql) //
 				.isEqualTo(
-						"SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", "
+						"SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" AS \"FIXEDCUSTOMPROPERTYPREFIX_ID\", "
-								+ "\"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_NAME\" AS \"FIXEDCUSTOMPROPERTYPREFIX_NAME\", "
+								+ "\"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\" "
+								+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" "
-								+ "LEFT OUTER JOIN \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" AS \"ref\" ON \"ref\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\" = \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" L"
+								+ "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\" "
+								+ "EFT OUTER JOIN \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" AS \"ref_further\" ON \"ref_further\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" = \"ref\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-								+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id");
+								+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_DUMMYENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_ID\" = :id");
 		softAssertions.assertAll();
 	}
 
@@ -122,8 +122,8 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 
 		String sql = sqlGenerator.createDeleteByPath(getPath("ref"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId");
+				+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId");
 	}
 
 	@Test // DATAJDBC-107
@@ -133,11 +133,12 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 
 		String sql = sqlGenerator.createDeleteByPath(getPath("ref.further"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
+		assertThat(sql)
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
+				.isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
-				+ "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
-				+ "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)");
+						+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" = :rootId)");
 	}
 
 	@Test // DATAJDBC-107
@@ -147,7 +148,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 
 		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 class SqlGeneratorFixedNamingStrategyUnitTests {
 
 		String sql = sqlGenerator.createDeleteAllSql(getPath("ref"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL");
+				+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL");
 	}
 
 	@Test // DATAJDBC-107
@@ -168,11 +169,12 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 
 		String sql = sqlGenerator.createDeleteAllSql(getPath("ref.further"));
 
-		assertThat(sql).isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
+		assertThat(sql)
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
+				.isEqualTo("DELETE FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\" "
-				+ "(SELECT \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_SECONDLEVELREFERENCEDENTITY\".\"REFERENCED_ENTITY\" IN "
-				+ "FROM \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "(SELECT \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"FIXEDCUSTOMPROPERTYPREFIX_L1ID\" "
-				+ "WHERE \"FIXEDCUSTOMSCHEMA.FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)");
+						+ "FROM \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\" "
+						+ "WHERE \"FIXEDCUSTOMSCHEMA\".\"FIXEDCUSTOMTABLEPREFIX_REFERENCEDENTITY\".\"DUMMY_ENTITY\" IS NOT NULL)");
 	}
 
 	@Test // DATAJDBC-113
@@ -183,7 +185,7 @@ public class SqlGeneratorFixedNamingStrategyUnitTests {
 		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/src/main/java/org/springframework/data/relational/core/mapping/CachingNamingStrategy.java

@@ -66,6 +66,7 @@ class CachingNamingStrategy implements NamingStrategy {
 	 * @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 @@ public interface NamingStrategy {
 		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 @@ class RelationalPersistentEntityImpl<T> extends BasicPersistentEntity<T, Relatio
 	 */
 	@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,7 +19,10 @@ import static org.assertj.core.api.Assertions.*;
 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}.
@@ -27,6 +30,7 @@ import org.springframework.data.annotation.Id;
  * @author Oliver Gierke
  * @author Kazuki Shimizu
  * @author Bastian Wilhelm
+ * @author Mark Paluch
  */
 public class RelationalPersistentEntityImplUnitTests {
 
@@ -56,6 +60,18 @@ public class RelationalPersistentEntityImplUnitTests {
 		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 @@ public class RelationalPersistentEntityImplUnitTests {
 	static class DummyEntityWithEmptyAnnotation {
 		@Id @Column() Long id;
 	}
+
+	enum NamingStrategyWithSchema implements NamingStrategy {
+		INSTANCE;
+
+		@Override
+		public String getSchema() {
+			return "my_schema";
+		}
+	}
 }