From 592f483699fda8ef5d6ba846162dffd54a922c94 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 29 Jan 2020 09:12:02 +0100 Subject: [PATCH] DATAJDBC-479 - Use SqlIdentifier in SQL AST. We now use SqlIdentifier to encapsulate names and aliases of tables, columns and functions. We now use proper delegation to ConditionVisitor to render a JOIN condition. Previously, we used toString() of Condition segments which rendered an approximation of the condition. ConditionVisitor applies RenderContext settings that consider identifier quoting and normalization strategies. Original pull request: #187. --- .../data/jdbc/core/convert/SqlContext.java | 24 ++-- .../data/jdbc/core/convert/SqlGenerator.java | 62 ++++---- .../jdbc/core/convert/SqlGeneratorSource.java | 6 +- .../jdbc/core/mapping/JdbcMappingContext.java | 5 +- .../convert/IdentifierProcessingAdapter.java | 46 ++++++ ...orContextBasedNamingStrategyUnitTests.java | 1 + .../SqlGeneratorEmbeddedUnitTests.java | 25 ++-- ...GeneratorFixedNamingStrategyUnitTests.java | 1 + .../core/convert/SqlGeneratorUnitTests.java | 44 +++--- .../jdbc/core/convert/UnquotedDialect.java | 46 ++++++ .../core/dialect/RenderContextFactory.java | 14 +- .../BasicRelationalPersistentProperty.java | 4 +- .../core/mapping/DerivedSqlIdentifier.java | 5 +- .../RelationalPersistentEntityImpl.java | 4 +- .../data/relational/core/sql/Aliased.java | 2 +- .../core/sql/AliasedExpression.java | 12 +- .../data/relational/core/sql/BindMarker.java | 4 +- .../data/relational/core/sql/Column.java | 58 +++++++- .../core/sql/DefaultSqlIdentifier.java | 5 +- .../data/relational/core/sql/Named.java | 2 +- .../relational/core/sql/SimpleFunction.java | 20 ++- .../relational/core/sql/SqlIdentifier.java | 11 ++ .../data/relational/core/sql/Table.java | 98 +++++++++++-- .../core/sql/render/ColumnVisitor.java | 16 ++- .../core/sql/render/ExpressionVisitor.java | 5 +- .../core/sql/render/FromTableVisitor.java | 4 +- .../core/sql/render/JoinVisitor.java | 18 ++- .../core/sql/render/NameRenderer.java | 132 ++++++++++++++++++ .../core/sql/render/NamingStrategies.java | 17 +-- .../core/sql/render/OrderByClauseVisitor.java | 2 +- .../core/sql/render/RenderContext.java | 10 ++ .../core/sql/render/RenderNamingStrategy.java | 9 +- .../core/sql/render/SelectListVisitor.java | 9 +- .../core/sql/render/SimpleRenderContext.java | 7 + .../core/sql/SelectBuilderUnitTests.java | 2 +- .../sql/render/SelectRendererUnitTests.java | 22 ++- 36 files changed, 606 insertions(+), 146 deletions(-) create mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java create mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NameRenderer.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java index c89218d36..d79d7cfb9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlContext.java @@ -18,10 +18,8 @@ package org.springframework.data.jdbc.core.convert; import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.sql.Column; -import org.springframework.data.relational.core.sql.SQL; -import org.springframework.data.relational.core.sql.Table; -import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.SqlIdentifier; +import org.springframework.data.relational.core.sql.Table; /** * Utility to get from path to SQL DSL elements. @@ -35,21 +33,19 @@ class SqlContext { private final RelationalPersistentEntity entity; private final Table table; - private final IdentifierProcessing identifierProcessing; - SqlContext(RelationalPersistentEntity entity, IdentifierProcessing identifierProcessing) { + SqlContext(RelationalPersistentEntity entity) { - this.identifierProcessing = identifierProcessing; this.entity = entity; - this.table = SQL.table(entity.getTableName().toSql(this.identifierProcessing)); + this.table = Table.create(entity.getTableName()); } Column getIdColumn() { - return table.column(entity.getIdColumn().toSql(identifierProcessing)); + return table.column(entity.getIdColumn()); } Column getVersionColumn() { - return table.column(entity.getRequiredVersionProperty().getColumnName().toSql(identifierProcessing)); + return table.column(entity.getRequiredVersionProperty().getColumnName()); } Table getTable() { @@ -59,17 +55,15 @@ class SqlContext { Table getTable(PersistentPropertyPathExtension path) { SqlIdentifier tableAlias = path.getTableAlias(); - Table table = SQL.table(path.getTableName().toSql(identifierProcessing)); - return tableAlias == null ? table : table.as(tableAlias.toSql(identifierProcessing)); + Table table = Table.create(path.getTableName()); + return tableAlias == null ? table : table.as(tableAlias); } Column getColumn(PersistentPropertyPathExtension path) { - return getTable(path).column(path.getColumnName().toSql(identifierProcessing)) - .as(path.getColumnAlias().toSql(identifierProcessing)); + return getTable(path).column(path.getColumnName()).as(path.getColumnAlias()); } Column getReverseColumn(PersistentPropertyPathExtension path) { - return getTable(path).column(path.getReverseColumnName().toSql(identifierProcessing)) - .as(path.getReverseColumnNameAlias().toSql(identifierProcessing)); + return getTable(path).column(path.getReverseColumnName()).as(path.getReverseColumnNameAlias()); } } 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 b14d5ba21..31700022f 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 @@ -35,6 +35,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.sql.*; +import org.springframework.data.relational.core.sql.render.RenderContext; import org.springframework.data.relational.core.sql.render.SqlRenderer; import org.springframework.data.relational.domain.Identifier; import org.springframework.data.util.Lazy; @@ -64,7 +65,7 @@ class SqlGenerator { private final JdbcConverter converter; private final RelationalPersistentEntity entity; private final MappingContext, RelationalPersistentProperty> mappingContext; - private final IdentifierProcessing identifierProcessing; + private final RenderContext renderContext; private final SqlContext sqlContext; private final SqlRenderer sqlRenderer; @@ -92,16 +93,15 @@ class SqlGenerator { * @param entity must not be {@literal null}. * @param dialect must not be {@literal null}. */ - SqlGenerator(RelationalMappingContext mappingContext, JdbcConverter converter, RelationalPersistentEntity entity, - Dialect dialect) { + SqlGenerator(RelationalMappingContext mappingContext, JdbcConverter converter,RelationalPersistentEntity entity, Dialect dialect) { this.mappingContext = mappingContext; this.converter = converter; this.entity = entity; - this.identifierProcessing = dialect.getIdentifierProcessing(); - this.sqlContext = new SqlContext(entity, this.identifierProcessing); + this.sqlContext = new SqlContext(entity); this.sqlRenderer = SqlRenderer.create(new RenderContextFactory(dialect).createRenderContext()); this.columns = new Columns(entity, mappingContext, converter); + this.renderContext = new RenderContextFactory(dialect).createRenderContext(); } /** @@ -125,10 +125,9 @@ class SqlGenerator { return rootCondition.apply(filterColumn); } - Table subSelectTable = SQL.table(parentPath.getTableName().toSql(identifierProcessing)); - Column idColumn = subSelectTable.column(parentPath.getIdColumnName().toSql(identifierProcessing)); - Column selectFilterColumn = subSelectTable - .column(parentPath.getEffectiveIdColumnName().toSql(identifierProcessing)); + Table subSelectTable = Table.create(parentPath.getTableName()); + Column idColumn = subSelectTable.column(parentPath.getIdColumnName()); + Column selectFilterColumn = subSelectTable.column(parentPath.getEffectiveIdColumnName()); Condition innerCondition; @@ -151,7 +150,7 @@ class SqlGenerator { } private BindMarker getBindMarker(SqlIdentifier columnName) { - return SQL.bindMarker(":" + parameterPattern.matcher(columnName.getReference(identifierProcessing)).replaceAll("")); + return SQL.bindMarker(":" + parameterPattern.matcher(renderReference(columnName)).replaceAll("")); } /** @@ -210,21 +209,19 @@ class SqlGenerator { Assert.isTrue(keyColumn != null || !ordered, "If the SQL statement should be ordered a keyColumn to order by must be provided."); + Table table = getTable(); + SelectBuilder.SelectWhere builder = selectBuilder( // keyColumn == null // ? Collections.emptyList() // - : Collections.singleton(keyColumn.toSql(identifierProcessing)) // + : Collections.singleton(keyColumn) // ); - Table table = getTable(); - Condition condition = buildConditionForBackReference(parentIdentifier, table); SelectBuilder.SelectWhereAndOr withWhereClause = builder.where(condition); Select select = ordered // - ? withWhereClause - .orderBy(table.column(keyColumn.toSql(identifierProcessing)).as(keyColumn.toSql(identifierProcessing))) - .build() // + ? withWhereClause.orderBy(table.column(keyColumn).as(keyColumn)).build() // : withWhereClause.build(); return render(select); @@ -235,8 +232,7 @@ class SqlGenerator { Condition condition = null; for (SqlIdentifier backReferenceColumn : parentIdentifier.toMap().keySet()) { - Condition newCondition = table.column(backReferenceColumn.toSql(identifierProcessing)) - .isEqualTo(getBindMarker(backReferenceColumn)); + Condition newCondition = table.column(backReferenceColumn).isEqualTo(getBindMarker(backReferenceColumn)); condition = condition == null ? newCondition : condition.and(newCondition); } @@ -372,7 +368,7 @@ class SqlGenerator { return selectBuilder(Collections.emptyList()); } - private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns) { + private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns) { Table table = getTable(); @@ -396,7 +392,7 @@ class SqlGenerator { } } - for (String keyColumn : keyColumns) { + for (SqlIdentifier keyColumn : keyColumns) { columnExpressions.add(table.column(keyColumn).as(keyColumn)); } @@ -485,8 +481,8 @@ class SqlGenerator { return new Join( // currentTable, // - currentTable.column(path.getReverseColumnName().toSql(identifierProcessing)), // - parentTable.column(idDefiningParentPath.getIdColumnName().toSql(identifierProcessing)) // + currentTable.column(path.getReverseColumnName()), // + parentTable.column(idDefiningParentPath.getIdColumnName()) // ); } @@ -526,14 +522,14 @@ class SqlGenerator { Table table = getTable(); - Set columnNamesForInsert = new TreeSet<>(Comparator.comparing(id -> id.toSql(identifierProcessing))); + Set columnNamesForInsert = new TreeSet<>(Comparator.comparing(SqlIdentifier::getReference)); columnNamesForInsert.addAll(columns.getInsertableColumns()); columnNamesForInsert.addAll(additionalColumns); InsertBuilder.InsertIntoColumnsAndValuesWithBuild insert = Insert.builder().into(table); for (SqlIdentifier cn : columnNamesForInsert) { - insert = insert.column(table.column(cn.toSql(identifierProcessing))); + insert = insert.column(table.column(cn)); } InsertBuilder.InsertValuesWithBuild insertWithValues = null; @@ -551,8 +547,7 @@ class SqlGenerator { private String createUpdateWithVersionSql() { Update update = createBaseUpdate() // - .and(getVersionColumn() - .isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER.getReference(identifierProcessing)))) // + .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + renderReference(VERSION_SQL_PARAMETER)))) // .build(); return render(update); @@ -565,7 +560,7 @@ class SqlGenerator { List assignments = columns.getUpdateableColumns() // .stream() // .map(columnName -> Assignments.value( // - table.column(columnName.toSql(identifierProcessing)), // + table.column(columnName), // getBindMarker(columnName))) // .collect(Collectors.toList()); @@ -582,8 +577,7 @@ class SqlGenerator { private String createDeleteByIdAndVersionSql() { Delete delete = createBaseDeleteById(getTable()) // - .and(getVersionColumn() - .isEqualTo(SQL.bindMarker(":" + VERSION_SQL_PARAMETER.getReference(identifierProcessing)))) // + .and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + renderReference(VERSION_SQL_PARAMETER)))) // .build(); return render(delete); @@ -591,19 +585,19 @@ class SqlGenerator { private DeleteBuilder.DeleteWhereAndOr createBaseDeleteById(Table table) { return Delete.builder().from(table) - .where(getIdColumn().isEqualTo(SQL.bindMarker(":" + ID_SQL_PARAMETER.getReference(identifierProcessing)))); + .where(getIdColumn().isEqualTo(SQL.bindMarker(":" + renderReference(ID_SQL_PARAMETER)))); } private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path, Function rootCondition) { - Table table = SQL.table(path.getTableName().toSql(identifierProcessing)); + Table table = Table.create(path.getTableName()); DeleteBuilder.DeleteWhere builder = Delete.builder() // .from(table); Delete delete; - Column filterColumn = table.column(path.getReverseColumnName().toSql(identifierProcessing)); + Column filterColumn = table.column(path.getReverseColumnName()); if (path.getLength() == 1) { @@ -659,6 +653,10 @@ class SqlGenerator { return sqlContext.getVersionColumn(); } + private String renderReference(SqlIdentifier identifier) { + return identifier.getReference(renderContext.getIdentifierProcessing()); + } + private List extractOrderByFields(Sort sort) { return sort.stream() .map(order -> OrderByField.from(Column.create(order.getProperty(), this.getTable()), order.getDirection())) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java index 05e68c9cc..f8352a85f 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGeneratorSource.java @@ -40,14 +40,16 @@ public class SqlGeneratorSource { private final Dialect dialect; /** - * @return the {@link Dialect} used by the created {@link SqlGenerator} instances. Guaranteed to be not {@literal null}. + * @return the {@link Dialect} used by the created {@link SqlGenerator} instances. Guaranteed to be not + * {@literal null}. */ public Dialect getDialect() { return dialect; } SqlGenerator getSqlGenerator(Class domainType) { - return CACHE.computeIfAbsent(domainType, t -> new SqlGenerator(context, converter, + return CACHE.computeIfAbsent(domainType, + t -> new SqlGenerator(context, converter, context.getRequiredPersistentEntity(t), dialect)); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/JdbcMappingContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/JdbcMappingContext.java index 85861d289..0fe33bc39 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/JdbcMappingContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/mapping/JdbcMappingContext.java @@ -85,7 +85,10 @@ public class JdbcMappingContext extends RelationalMappingContext { @Override protected RelationalPersistentProperty createPersistentProperty(Property property, RelationalPersistentEntity owner, SimpleTypeHolder simpleTypeHolder) { - return new BasicJdbcPersistentProperty(property, owner, simpleTypeHolder, this.getNamingStrategy()); + BasicJdbcPersistentProperty persistentProperty = new BasicJdbcPersistentProperty(property, owner, simpleTypeHolder, + this.getNamingStrategy()); + persistentProperty.setForceQuote(isForceQuote()); + return persistentProperty; } @Override diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java new file mode 100644 index 000000000..a20be2c00 --- /dev/null +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierProcessingAdapter.java @@ -0,0 +1,46 @@ +/* + * Copyright 2020 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.jdbc.core.convert; + +import org.springframework.data.relational.core.dialect.AbstractDialect; +import org.springframework.data.relational.core.dialect.Dialect; +import org.springframework.data.relational.core.dialect.HsqlDbDialect; +import org.springframework.data.relational.core.dialect.LimitClause; +import org.springframework.data.relational.core.sql.IdentifierProcessing; + +/** + * {@link Dialect} adapter that delegates to the given {@link IdentifierProcessing}. + * + * @author Mark Paluch + */ +public class IdentifierProcessingAdapter extends AbstractDialect implements Dialect { + + private final IdentifierProcessing identifierProcessing; + + public IdentifierProcessingAdapter(IdentifierProcessing identifierProcessing) { + this.identifierProcessing = identifierProcessing; + } + + @Override + public LimitClause limit() { + return HsqlDbDialect.INSTANCE.limit(); + } + + @Override + public IdentifierProcessing getIdentifierProcessing() { + return identifierProcessing; + } +} diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java index c1ad3ebe5..7bcc13d9e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorContextBasedNamingStrategyUnitTests.java @@ -24,6 +24,7 @@ import java.util.function.Consumer; import org.assertj.core.api.SoftAssertions; import org.junit.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java index f9e37568a..15cb6d777 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorEmbeddedUnitTests.java @@ -22,6 +22,7 @@ import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.PropertyPathTestingUtils; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; @@ -33,6 +34,7 @@ import org.springframework.data.relational.core.mapping.PersistentPropertyPathEx import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.sql.Aliased; +import org.springframework.data.relational.core.sql.SqlIdentifier; /** * Unit tests for the {@link SqlGenerator} in a context of the {@link Embedded} annotation. @@ -49,6 +51,7 @@ public class SqlGeneratorEmbeddedUnitTests { @Before public void setUp() { + this.context.setForceQuote(false); this.sqlGenerator = createSqlGenerator(DummyEntity.class); } @@ -201,7 +204,8 @@ public class SqlGeneratorEmbeddedUnitTests { assertThat(generatedColumn("embeddable.test", DummyEntity.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly("test", "dummy_entity", null, "test"); + .containsExactly(SqlIdentifier.unquoted("test"), SqlIdentifier.unquoted("dummy_entity"), null, + SqlIdentifier.unquoted("test")); } @Test // DATAJDBC-340 @@ -224,7 +228,8 @@ public class SqlGeneratorEmbeddedUnitTests { assertThat(generatedColumn("prefixedEmbeddable.test", DummyEntity.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly("prefix_test", "dummy_entity", null, "prefix_test"); + .containsExactly(SqlIdentifier.unquoted("prefix_test"), SqlIdentifier.unquoted("dummy_entity"), null, + SqlIdentifier.unquoted("prefix_test")); } @Test // DATAJDBC-340 @@ -240,7 +245,8 @@ public class SqlGeneratorEmbeddedUnitTests { assertThat(generatedColumn("embeddable.embeddable.attr1", DummyEntity.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly("attr1", "dummy_entity", null, "attr1"); + .containsExactly(SqlIdentifier.unquoted("attr1"), SqlIdentifier.unquoted("dummy_entity"), null, + SqlIdentifier.unquoted("attr1")); } @Test // DATAJDBC-340 @@ -250,11 +256,11 @@ public class SqlGeneratorEmbeddedUnitTests { SoftAssertions.assertSoftly(softly -> { - softly.assertThat(join.getJoinTable().getName()).isEqualTo("other_entity"); + softly.assertThat(join.getJoinTable().getName()).isEqualTo(SqlIdentifier.unquoted("other_entity")); softly.assertThat(join.getJoinColumn().getTable()).isEqualTo(join.getJoinTable()); - softly.assertThat(join.getJoinColumn().getName()).isEqualTo("dummy_entity2"); - softly.assertThat(join.getParentId().getName()).isEqualTo("id"); - softly.assertThat(join.getParentId().getTable().getName()).isEqualTo("dummy_entity2"); + softly.assertThat(join.getJoinColumn().getName()).isEqualTo(SqlIdentifier.unquoted("dummy_entity2")); + softly.assertThat(join.getParentId().getName()).isEqualTo(SqlIdentifier.unquoted("id")); + softly.assertThat(join.getParentId().getTable().getName()).isEqualTo(SqlIdentifier.unquoted("dummy_entity2")); }); } @@ -263,7 +269,8 @@ public class SqlGeneratorEmbeddedUnitTests { assertThat(generatedColumn("embedded.other.value", DummyEntity2.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly("value", "other_entity", "prefix_other", "prefix_other_value"); + .containsExactly(SqlIdentifier.unquoted("value"), SqlIdentifier.unquoted("other_entity"), + SqlIdentifier.quoted("prefix_other"), SqlIdentifier.unquoted("prefix_other_value")); } private SqlGenerator.Join generateJoin(String path, Class type) { @@ -271,7 +278,7 @@ public class SqlGeneratorEmbeddedUnitTests { .getJoin(new PersistentPropertyPathExtension(context, PropertyPathTestingUtils.toPath(path, type, context))); } - private String getAlias(Object maybeAliased) { + private SqlIdentifier getAlias(Object maybeAliased) { if (maybeAliased instanceof Aliased) { return ((Aliased) maybeAliased).getAlias(); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java index 4c0d09d47..40955405e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorFixedNamingStrategyUnitTests.java @@ -25,6 +25,7 @@ import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.core.mapping.PersistentPropertyPathTestUtils; import org.springframework.data.jdbc.testing.AnsiDialect; import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.relational.core.dialect.HsqlDbDialect; import org.springframework.data.relational.core.mapping.NamingStrategy; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; 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 c7b759d28..8b81be2e4 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 @@ -47,6 +47,10 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext 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.IdentifierProcessing.LetterCasing; +import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.data.relational.domain.Identifier; @@ -555,11 +559,11 @@ public class SqlGeneratorUnitTests { SoftAssertions.assertSoftly(softly -> { - softly.assertThat(join.getJoinTable().getName()).isEqualTo("\"REFERENCED_ENTITY\""); + softly.assertThat(join.getJoinTable().getName()).isEqualTo(SqlIdentifier.quoted("REFERENCED_ENTITY")); softly.assertThat(join.getJoinColumn().getTable()).isEqualTo(join.getJoinTable()); - softly.assertThat(join.getJoinColumn().getName()).isEqualTo("\"DUMMY_ENTITY\""); - softly.assertThat(join.getParentId().getName()).isEqualTo("\"id1\""); - softly.assertThat(join.getParentId().getTable().getName()).isEqualTo("\"DUMMY_ENTITY\""); + softly.assertThat(join.getJoinColumn().getName()).isEqualTo(SqlIdentifier.quoted("DUMMY_ENTITY")); + softly.assertThat(join.getParentId().getName()).isEqualTo(SqlIdentifier.quoted("id1")); + softly.assertThat(join.getParentId().getTable().getName()).isEqualTo(SqlIdentifier.quoted("DUMMY_ENTITY")); }); } @@ -587,11 +591,12 @@ public class SqlGeneratorUnitTests { SoftAssertions.assertSoftly(softly -> { - softly.assertThat(join.getJoinTable().getName()).isEqualTo("\"SECOND_LEVEL_REFERENCED_ENTITY\""); + softly.assertThat(join.getJoinTable().getName()) + .isEqualTo(SqlIdentifier.quoted("SECOND_LEVEL_REFERENCED_ENTITY")); softly.assertThat(join.getJoinColumn().getTable()).isEqualTo(join.getJoinTable()); - softly.assertThat(join.getJoinColumn().getName()).isEqualTo("\"REFERENCED_ENTITY\""); - softly.assertThat(join.getParentId().getName()).isEqualTo("\"X_L1ID\""); - softly.assertThat(join.getParentId().getTable().getName()).isEqualTo("\"REFERENCED_ENTITY\""); + softly.assertThat(join.getJoinColumn().getName()).isEqualTo(SqlIdentifier.quoted("REFERENCED_ENTITY")); + softly.assertThat(join.getParentId().getName()).isEqualTo(SqlIdentifier.quoted("X_L1ID")); + softly.assertThat(join.getParentId().getTable().getName()).isEqualTo(SqlIdentifier.quoted("REFERENCED_ENTITY")); }); } @@ -603,13 +608,14 @@ public class SqlGeneratorUnitTests { SoftAssertions.assertSoftly(softly -> { - softly.assertThat(joinTable.getName()).isEqualTo("\"NO_ID_CHILD\""); + softly.assertThat(joinTable.getName()).isEqualTo(SqlIdentifier.quoted("NO_ID_CHILD")); softly.assertThat(joinTable).isInstanceOf(Aliased.class); - softly.assertThat(((Aliased) joinTable).getAlias()).isEqualTo("\"child\""); + softly.assertThat(((Aliased) joinTable).getAlias()).isEqualTo(SqlIdentifier.quoted("child")); softly.assertThat(join.getJoinColumn().getTable()).isEqualTo(joinTable); - softly.assertThat(join.getJoinColumn().getName()).isEqualTo("\"PARENT_OF_NO_ID_CHILD\""); - softly.assertThat(join.getParentId().getName()).isEqualTo("\"X_ID\""); - softly.assertThat(join.getParentId().getTable().getName()).isEqualTo("\"PARENT_OF_NO_ID_CHILD\""); + softly.assertThat(join.getJoinColumn().getName()).isEqualTo(SqlIdentifier.quoted("PARENT_OF_NO_ID_CHILD")); + softly.assertThat(join.getParentId().getName()).isEqualTo(SqlIdentifier.quoted("X_ID")); + softly.assertThat(join.getParentId().getTable().getName()) + .isEqualTo(SqlIdentifier.quoted("PARENT_OF_NO_ID_CHILD")); }); } @@ -624,7 +630,8 @@ public class SqlGeneratorUnitTests { assertThat(generatedColumn("id", DummyEntity.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) - .containsExactly("\"id1\"", "\"DUMMY_ENTITY\"", null, "\"id1\""); + .containsExactly(SqlIdentifier.quoted("id1"), SqlIdentifier.quoted("DUMMY_ENTITY"), null, + SqlIdentifier.quoted("id1")); } @Test // DATAJDBC-340 @@ -632,7 +639,8 @@ public class SqlGeneratorUnitTests { assertThat(generatedColumn("ref.l1id", DummyEntity.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) // - .containsExactly("\"X_L1ID\"", "\"REFERENCED_ENTITY\"", "\"ref\"", "\"REF_X_L1ID\""); + .containsExactly(SqlIdentifier.quoted("X_L1ID"), SqlIdentifier.quoted("REFERENCED_ENTITY"), + SqlIdentifier.quoted("ref"), SqlIdentifier.quoted("REF_X_L1ID")); } @Test // DATAJDBC-340 @@ -646,11 +654,11 @@ public class SqlGeneratorUnitTests { assertThat(generatedColumn("child", ParentOfNoIdChild.class)) // .extracting(c -> c.getName(), c -> c.getTable().getName(), c -> getAlias(c.getTable()), this::getAlias) // - .containsExactly("\"PARENT_OF_NO_ID_CHILD\"", "\"NO_ID_CHILD\"", "\"child\"", - "\"CHILD_PARENT_OF_NO_ID_CHILD\""); + .containsExactly(SqlIdentifier.quoted("PARENT_OF_NO_ID_CHILD"), SqlIdentifier.quoted("NO_ID_CHILD"), + SqlIdentifier.quoted("child"), SqlIdentifier.quoted("CHILD_PARENT_OF_NO_ID_CHILD")); } - private String getAlias(Object maybeAliased) { + private SqlIdentifier getAlias(Object maybeAliased) { if (maybeAliased instanceof Aliased) { return ((Aliased) maybeAliased).getAlias(); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java new file mode 100644 index 000000000..835517790 --- /dev/null +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/UnquotedDialect.java @@ -0,0 +1,46 @@ +/* + * Copyright 2020 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.jdbc.core.convert; + +import org.springframework.data.relational.core.dialect.AbstractDialect; +import org.springframework.data.relational.core.dialect.Dialect; +import org.springframework.data.relational.core.dialect.HsqlDbDialect; +import org.springframework.data.relational.core.dialect.LimitClause; +import org.springframework.data.relational.core.sql.IdentifierProcessing; +import org.springframework.data.relational.core.sql.IdentifierProcessing.LetterCasing; +import org.springframework.data.relational.core.sql.IdentifierProcessing.Quoting; + +/** + * Simple {@link Dialect} that provides unquoted {@link IdentifierProcessing}. + * + * @author Mark Paluch + */ +public class UnquotedDialect extends AbstractDialect implements Dialect { + + public static final UnquotedDialect INSTANCE = new UnquotedDialect(); + + private UnquotedDialect() {} + + @Override + public LimitClause limit() { + return HsqlDbDialect.INSTANCE.limit(); + } + + @Override + public IdentifierProcessing getIdentifierProcessing() { + return IdentifierProcessing.create(new Quoting(""), LetterCasing.AS_IS); + } +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java index 92fdba640..20b576fe2 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/RenderContextFactory.java @@ -17,6 +17,7 @@ package org.springframework.data.relational.core.dialect; import lombok.RequiredArgsConstructor; +import org.springframework.data.relational.core.sql.IdentifierProcessing; import org.springframework.data.relational.core.sql.render.NamingStrategies; import org.springframework.data.relational.core.sql.render.RenderContext; import org.springframework.data.relational.core.sql.render.RenderNamingStrategy; @@ -68,7 +69,7 @@ public class RenderContextFactory { SelectRenderContext select = dialect.getSelectContext(); - return new DialectRenderContext(namingStrategy, select); + return new DialectRenderContext(namingStrategy, dialect.getIdentifierProcessing(), select); } /** @@ -79,6 +80,8 @@ public class RenderContextFactory { private final RenderNamingStrategy renderNamingStrategy; + private final IdentifierProcessing identifierProcessing; + private final SelectRenderContext selectRenderContext; /* @@ -90,6 +93,15 @@ public class RenderContextFactory { return renderNamingStrategy; } + /* + * (non-Javadoc) + * @see org.springframework.data.relational.core.sql.render.RenderContext#getIdentifierProcessing() + */ + @Override + public IdentifierProcessing getIdentifierProcessing() { + return identifierProcessing; + } + /* * (non-Javadoc) * @see org.springframework.data.relational.core.sql.render.RenderContext#getSelect() diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java index a77286e18..68bd4896a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/BasicRelationalPersistentProperty.java @@ -128,11 +128,11 @@ public class BasicRelationalPersistentProperty extends AnnotationBasedPersistent throw new UnsupportedOperationException(); } - boolean isForceQuote() { + public boolean isForceQuote() { return forceQuote; } - void setForceQuote(boolean forceQuote) { + public void setForceQuote(boolean forceQuote) { this.forceQuote = forceQuote; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DerivedSqlIdentifier.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DerivedSqlIdentifier.java index b9ac43724..34dcde8f3 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DerivedSqlIdentifier.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/DerivedSqlIdentifier.java @@ -80,11 +80,14 @@ class DerivedSqlIdentifier implements SqlIdentifier { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; + } + if (o instanceof SqlIdentifier) { return toString().equals(o.toString()); } + return false; } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java index 9e5360aec..262e5d830 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/RelationalPersistentEntityImpl.java @@ -64,11 +64,11 @@ class RelationalPersistentEntityImpl extends BasicPersistentEntity expressions, String alias) { + AliasedFunction(String functionName, List expressions, SqlIdentifier alias) { super(functionName, expressions); this.alias = alias; } @@ -109,7 +123,7 @@ public class SimpleFunction extends AbstractSegment implements Expression { * @see org.springframework.data.relational.core.sql.Aliased#getAlias() */ @Override - public String getAlias() { + public SqlIdentifier getAlias() { return alias; } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlIdentifier.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlIdentifier.java index 39e267968..f3d43b0c4 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlIdentifier.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/SqlIdentifier.java @@ -69,6 +69,17 @@ public interface SqlIdentifier { */ String getReference(IdentifierProcessing processing); + /** + * Return the reference name without any further transformation. The reference name is used for programmatic access to + * the object identified by this {@link SqlIdentifier}. + * + * @return + * @see IdentifierProcessing#NONE + */ + default String getReference() { + return getReference(IdentifierProcessing.NONE); + } + /** * Return the identifier for SQL usage after applying {@link IdentifierProcessing} rules. The identifier name is used * to construct SQL statements. diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java index e371f69f4..98dad4339 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java @@ -33,9 +33,14 @@ import org.springframework.util.Assert; */ public class Table extends AbstractSegment { - private final String name; + private final SqlIdentifier name; Table(String name) { + super(); + this.name = SqlIdentifier.unquoted(name); + } + + Table(SqlIdentifier name) { super(); this.name = name; } @@ -48,7 +53,21 @@ public class Table extends AbstractSegment { */ public static Table create(String name) { - Assert.hasText(name, "Name must not be null or empty!"); + Assert.hasText(name, "Name must not be null or empty"); + + return new Table(name); + } + + /** + * Creates a new {@link Table} given {@code name}. + * + * @param name must not be {@literal null} or empty. + * @return the new {@link Table}. + * @since 2.0 + */ + public static Table create(SqlIdentifier name) { + + Assert.notNull(name, "Name must not be null"); return new Table(name); } @@ -78,6 +97,20 @@ public class Table extends AbstractSegment { Assert.hasText(alias, "Alias must not be null or empty!"); + return new AliasedTable(name, SqlIdentifier.unquoted(alias)); + } + + /** + * Creates a new {@link Table} aliased to {@code alias}. + * + * @param alias must not be {@literal null} or empty. + * @return the new {@link Table} using the {@code alias}. + * @since 2.0 + */ + public Table as(SqlIdentifier alias) { + + Assert.notNull(alias, "Alias must not be null"); + return new AliasedTable(name, alias); } @@ -97,6 +130,23 @@ public class Table extends AbstractSegment { return new Column(name, this); } + /** + * Creates a new {@link Column} associated with this {@link Table}. + *

+ * Note: This {@link Table} does not track column creation and there is no possibility to enumerate all + * {@link Column}s that were created for this table. + * + * @param name column name, must not be {@literal null} or empty. + * @return a new {@link Column} associated with this {@link Table}. + * @since 2.0 + */ + public Column column(SqlIdentifier name) { + + Assert.notNull(name, "Name must not be null"); + + return new Column(name, this); + } + /** * Creates a {@link List} of {@link Column}s associated with this {@link Table}. *

@@ -113,6 +163,28 @@ public class Table extends AbstractSegment { return columns(Arrays.asList(names)); } + /** + * Creates a {@link List} of {@link Column}s associated with this {@link Table}. + *

+ * Note: This {@link Table} does not track column creation and there is no possibility to enumerate all + * {@link Column}s that were created for this table. + * + * @param names column names, must not be {@literal null} or empty. + * @return a new {@link List} of {@link Column}s associated with this {@link Table}. + * @since 2.0 + */ + public List columns(SqlIdentifier... names) { + + Assert.notNull(names, "Names must not be null"); + + List columns = new ArrayList<>(); + for (SqlIdentifier name : names) { + columns.add(column(name)); + } + + return columns; + } + /** * Creates a {@link List} of {@link Column}s associated with this {@link Table}. *

@@ -153,7 +225,7 @@ public class Table extends AbstractSegment { * (non-Javadoc) * @see org.springframework.data.relational.core.sql.Named#getName() */ - public String getName() { + public SqlIdentifier getName() { return name; } @@ -161,7 +233,7 @@ public class Table extends AbstractSegment { * @return the table name as it is used in references. This can be the actual {@link #getName() name} or an * {@link Aliased#getAlias() alias}. */ - public String getReferenceName() { + public SqlIdentifier getReferenceName() { return name; } @@ -171,7 +243,7 @@ public class Table extends AbstractSegment { */ @Override public String toString() { - return name; + return name.toString(); } /** @@ -179,12 +251,20 @@ public class Table extends AbstractSegment { */ static class AliasedTable extends Table implements Aliased { - private final String alias; + private final SqlIdentifier alias; AliasedTable(String name, String alias) { super(name); - Assert.hasText(alias, "Alias must not be null or empty!"); + Assert.hasText(alias, "Alias must not be null or empty"); + + this.alias = SqlIdentifier.unquoted(alias); + } + + AliasedTable(SqlIdentifier name, SqlIdentifier alias) { + super(name); + + Assert.notNull(alias, "Alias must not be null"); this.alias = alias; } @@ -194,7 +274,7 @@ public class Table extends AbstractSegment { * @see org.springframework.data.relational.core.sql.Aliased#getAlias() */ @Override - public String getAlias() { + public SqlIdentifier getAlias() { return alias; } @@ -203,7 +283,7 @@ public class Table extends AbstractSegment { * @see org.springframework.data.relational.core.sql.Table#getReferenceName() */ @Override - public String getReferenceName() { + public SqlIdentifier getReferenceName() { return getAlias(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java index 27e292708..6ac108cdb 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ColumnVisitor.java @@ -16,6 +16,7 @@ package org.springframework.data.relational.core.sql.render; import org.springframework.data.relational.core.sql.Column; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.data.relational.core.sql.Visitable; import org.springframework.lang.Nullable; @@ -32,7 +33,7 @@ class ColumnVisitor extends TypedSubtreeVisitor { private final RenderTarget target; private final boolean considerTablePrefix; - private @Nullable String tableName; + private @Nullable SqlIdentifier tableName; ColumnVisitor(RenderContext context, boolean considerTablePrefix, RenderTarget target) { this.context = context; @@ -47,13 +48,14 @@ class ColumnVisitor extends TypedSubtreeVisitor { @Override Delegation leaveMatched(Column segment) { - String column = context.getNamingStrategy().getName(segment); - StringBuilder builder = new StringBuilder( - tableName != null ? tableName.length() + column.length() : column.length()); + SqlIdentifier column = context.getNamingStrategy().getName(segment); + StringBuilder builder = new StringBuilder(); if (considerTablePrefix && tableName != null) { - builder.append(tableName); + + builder.append(NameRenderer.render(context, SqlIdentifier.from(tableName, column))); + } else { + builder.append(NameRenderer.render(context, segment)); } - builder.append(column); target.onRendered(builder); return super.leaveMatched(segment); @@ -67,7 +69,7 @@ class ColumnVisitor extends TypedSubtreeVisitor { Delegation leaveNested(Visitable segment) { if (segment instanceof Table) { - tableName = context.getNamingStrategy().getReferenceName((Table) segment) + '.'; + tableName = context.getNamingStrategy().getReferenceName((Table) segment); } return super.leaveNested(segment); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ExpressionVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ExpressionVisitor.java index 71ca84639..408da4df2 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ExpressionVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/ExpressionVisitor.java @@ -61,14 +61,13 @@ class ExpressionVisitor extends TypedSubtreeVisitor implements PartR if (segment instanceof Column) { - RenderNamingStrategy namingStrategy = context.getNamingStrategy(); Column column = (Column) segment; - value = namingStrategy.getReferenceName(column.getTable()) + "." + namingStrategy.getReferenceName(column); + value = NameRenderer.fullyQualifiedReference(context, column); } else if (segment instanceof BindMarker) { if (segment instanceof Named) { - value = ((Named) segment).getName(); + value = NameRenderer.render(context, (Named) segment); } else { value = segment.toString(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/FromTableVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/FromTableVisitor.java index 623f9b4a1..93d8a470a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/FromTableVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/FromTableVisitor.java @@ -47,9 +47,9 @@ class FromTableVisitor extends TypedSubtreeVisitor { StringBuilder builder = new StringBuilder(); - builder.append(context.getNamingStrategy().getName(segment)); + builder.append(NameRenderer.render(context, segment)); if (segment instanceof Aliased) { - builder.append(" ").append(((Aliased) segment).getAlias()); + builder.append(" ").append(NameRenderer.render(context, (Aliased) segment)); } parent.onRendered(builder); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java index 135606930..e5934b586 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/JoinVisitor.java @@ -33,12 +33,14 @@ class JoinVisitor extends TypedSubtreeVisitor { private final RenderContext context; private final RenderTarget parent; private final StringBuilder joinClause = new StringBuilder(); + private final ConditionVisitor conditionVisitor; private boolean inCondition = false; private boolean hasSeenCondition = false; JoinVisitor(RenderContext context, RenderTarget parent) { this.context = context; this.parent = parent; + this.conditionVisitor = new ConditionVisitor(context); } /* @@ -61,18 +63,16 @@ class JoinVisitor extends TypedSubtreeVisitor { Delegation enterNested(Visitable segment) { if (segment instanceof Table && !inCondition) { - joinClause.append(context.getNamingStrategy().getName(((Table) segment))); + joinClause.append(NameRenderer.render(context, (Table) segment)); if (segment instanceof Aliased) { - joinClause.append(" AS ").append(((Aliased) segment).getAlias()); + joinClause.append(" AS ").append(NameRenderer.render(context, (Aliased) segment)); } } else if (segment instanceof Condition) { - // TODO: Use proper delegation for condition rendering. inCondition = true; if (!hasSeenCondition) { hasSeenCondition = true; - joinClause.append(" ON "); - joinClause.append(segment); + return Delegation.delegateTo(conditionVisitor); } } @@ -88,6 +88,14 @@ class JoinVisitor extends TypedSubtreeVisitor { if (segment instanceof Condition) { inCondition = false; + + if (hasSeenCondition) { + + joinClause.append(" ON "); + joinClause.append(conditionVisitor.getRenderedPart()); + + hasSeenCondition = false; + } } return super.leaveNested(segment); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NameRenderer.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NameRenderer.java new file mode 100644 index 000000000..22a6a7e33 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NameRenderer.java @@ -0,0 +1,132 @@ +/* + * Copyright 2020 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.sql.render; + +import org.springframework.data.relational.core.sql.Aliased; +import org.springframework.data.relational.core.sql.Column; +import org.springframework.data.relational.core.sql.IdentifierProcessing; +import org.springframework.data.relational.core.sql.Named; +import org.springframework.data.relational.core.sql.SqlIdentifier; +import org.springframework.data.relational.core.sql.Table; + +/** + * Utility to render {@link Column} and {@link Table} names using {@link SqlIdentifier} and {@link RenderContext} to + * SQL. + * + * @author Mark Paluch + */ +class NameRenderer { + + /** + * Render the {@link Table#getName() table name } with considering the {@link RenderNamingStrategy#getName(Table) + * naming strategy}. + * + * @param context + * @param table + * @return + */ + static CharSequence render(RenderContext context, Table table) { + return render(context, context.getNamingStrategy().getName(table)); + } + + /** + * Render the {@link Column#getName() column name} with considering the {@link RenderNamingStrategy#getName(Column) + * naming strategy}. + * + * @param context + * @param table + * @return + */ + static CharSequence render(RenderContext context, Column column) { + return render(context, context.getNamingStrategy().getName(column)); + } + + /** + * Render the {@link Named#getName() name}. + * + * @param context + * @param table + * @return + */ + static CharSequence render(RenderContext context, Named named) { + return render(context, named.getName()); + } + + /** + * Render the {@link Aliased#getAlias() alias}. + * + * @param context + * @param table + * @return + */ + static CharSequence render(RenderContext context, Aliased aliased) { + return render(context, aliased.getAlias()); + } + + /** + * Render the {@link Table#getReferenceName()} table reference name} with considering the + * {@link RenderNamingStrategy#getReferenceName(Table) naming strategy}. + * + * @param context + * @param table + * @return + */ + static CharSequence reference(RenderContext context, Table table) { + return render(context, context.getNamingStrategy().getReferenceName(table)); + } + + /** + * Render the {@link Column#getReferenceName()} column reference name} with considering the + * {@link RenderNamingStrategy#getReferenceName(Column) naming strategy}. + * + * @param context + * @param table + * @return + */ + static CharSequence reference(RenderContext context, Column column) { + return render(context, context.getNamingStrategy().getReferenceName(column)); + } + + /** + * Render the fully-qualified table and column name with considering the naming strategies of each component. + * + * @param context + * @param column + * @return + * @see RenderNamingStrategy#getReferenceName + */ + static CharSequence fullyQualifiedReference(RenderContext context, Column column) { + + RenderNamingStrategy namingStrategy = context.getNamingStrategy(); + + return render(context, SqlIdentifier.from(namingStrategy.getReferenceName(column.getTable()), + namingStrategy.getReferenceName(column))); + } + + /** + * Render the {@link SqlIdentifier#toSql(IdentifierProcessing) identifier to SQL} considering + * {@link IdentifierProcessing}. + * + * @param context + * @param identifier + * @return + */ + static CharSequence render(RenderContext context, SqlIdentifier identifier) { + return identifier.toSql(context.getIdentifierProcessing()); + } + + private NameRenderer() {} +} diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NamingStrategies.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NamingStrategies.java index 7cd4ed0c8..c9b4b5669 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NamingStrategies.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/NamingStrategies.java @@ -21,6 +21,7 @@ import java.util.Locale; import java.util.function.Function; import org.springframework.data.relational.core.sql.Column; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.util.Assert; @@ -121,23 +122,23 @@ public abstract class NamingStrategies { private final Function mappingFunction; @Override - public String getName(Column column) { - return mappingFunction.apply(delegate.getName(column)); + public SqlIdentifier getName(Column column) { + return delegate.getName(column).transform(mappingFunction::apply); } @Override - public String getReferenceName(Column column) { - return mappingFunction.apply(delegate.getReferenceName(column)); + public SqlIdentifier getReferenceName(Column column) { + return delegate.getReferenceName(column).transform(mappingFunction::apply); } @Override - public String getName(Table table) { - return mappingFunction.apply(delegate.getName(table)); + public SqlIdentifier getName(Table table) { + return delegate.getName(table).transform(mappingFunction::apply); } @Override - public String getReferenceName(Table table) { - return mappingFunction.apply(delegate.getReferenceName(table)); + public SqlIdentifier getReferenceName(Table table) { + return delegate.getReferenceName(table).transform(mappingFunction::apply); } } } 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 9f3e7b3de..1916bf2c3 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 @@ -77,7 +77,7 @@ class OrderByClauseVisitor extends TypedSubtreeVisitor implements Delegation leaveNested(Visitable segment) { if (segment instanceof Column) { - builder.append(context.getNamingStrategy().getReferenceName(((Column) segment))); + builder.append(NameRenderer.reference(context, (Column) segment)); } return super.leaveNested(segment); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java index d489d605e..011175d6a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderContext.java @@ -15,6 +15,8 @@ */ package org.springframework.data.relational.core.sql.render; +import org.springframework.data.relational.core.sql.IdentifierProcessing; + /** * Render context providing {@link RenderNamingStrategy} and other resources that are required during rendering. * @@ -30,6 +32,14 @@ public interface RenderContext { */ RenderNamingStrategy getNamingStrategy(); + /** + * Returns the configured {@link IdentifierProcessing}. + * + * @return the {@link IdentifierProcessing}. + * @since 2.0 + */ + IdentifierProcessing getIdentifierProcessing(); + /** * @return the {@link SelectRenderContext}. */ diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderNamingStrategy.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderNamingStrategy.java index 3bf18df6f..50b12bf56 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderNamingStrategy.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/RenderNamingStrategy.java @@ -18,6 +18,7 @@ package org.springframework.data.relational.core.sql.render; import java.util.function.Function; import org.springframework.data.relational.core.sql.Column; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.data.relational.core.sql.render.NamingStrategies.DelegatingRenderNamingStrategy; import org.springframework.util.Assert; @@ -38,7 +39,7 @@ public interface RenderNamingStrategy { * @return the {@link Column#getName() column name}. * @see Column#getName() */ - default String getName(Column column) { + default SqlIdentifier getName(Column column) { return column.getName(); } @@ -49,7 +50,7 @@ public interface RenderNamingStrategy { * @return the {@link Column#getName() column reference name}. * @see Column#getReferenceName() () */ - default String getReferenceName(Column column) { + default SqlIdentifier getReferenceName(Column column) { return column.getReferenceName(); } @@ -60,7 +61,7 @@ public interface RenderNamingStrategy { * @return the {@link Table#getName() table name}. * @see Table#getName() */ - default String getName(Table table) { + default SqlIdentifier getName(Table table) { return table.getName(); } @@ -71,7 +72,7 @@ public interface RenderNamingStrategy { * @return the {@link Table#getReferenceName() table name}. * @see Table#getReferenceName() */ - default String getReferenceName(Table table) { + default SqlIdentifier getReferenceName(Table table) { return table.getReferenceName(); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectListVisitor.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectListVisitor.java index 290ca0b82..420bcbfb8 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectListVisitor.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SelectListVisitor.java @@ -40,7 +40,6 @@ class SelectListVisitor extends TypedSubtreeVisitor implements PartR private boolean insideFunction = false; // this is hackery and should be fix with a proper visitor for // subelements. - SelectListVisitor(RenderContext context, RenderTarget target) { this.context = context; this.target = target; @@ -84,14 +83,14 @@ class SelectListVisitor extends TypedSubtreeVisitor implements PartR Delegation leaveNested(Visitable segment) { if (segment instanceof Table) { - builder.append(context.getNamingStrategy().getReferenceName((Table) segment)).append('.'); + builder.append(NameRenderer.reference(context, (Table) segment)).append('.'); } if (segment instanceof SimpleFunction) { builder.append(")"); if (segment instanceof Aliased) { - builder.append(" AS ").append(((Aliased) segment).getAlias()); + builder.append(" AS ").append(NameRenderer.render(context, (Aliased) segment)); } insideFunction = false; @@ -101,9 +100,9 @@ class SelectListVisitor extends TypedSubtreeVisitor implements PartR requiresComma = true; } else if (segment instanceof Column) { - builder.append(context.getNamingStrategy().getName((Column) segment)); + builder.append(NameRenderer.render(context, (Column) segment)); if (segment instanceof Aliased && !insideFunction) { - builder.append(" AS ").append(((Aliased) segment).getAlias()); + builder.append(" AS ").append(NameRenderer.render(context, (Aliased) segment)); } requiresComma = true; } else if (segment instanceof AsteriskFromTable) { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java index 8edffe020..cfbaf970f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/SimpleRenderContext.java @@ -17,6 +17,8 @@ package org.springframework.data.relational.core.sql.render; import lombok.Value; +import org.springframework.data.relational.core.sql.IdentifierProcessing; + /** * Default {@link RenderContext} implementation. * @@ -28,6 +30,11 @@ class SimpleRenderContext implements RenderContext { private final RenderNamingStrategy namingStrategy; + @Override + public IdentifierProcessing getIdentifierProcessing() { + return IdentifierProcessing.NONE; + } + @Override public SelectRenderContext getSelect() { return DefaultSelectRenderContext.INSTANCE; diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java index 1d88a9672..a71f92670 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/SelectBuilderUnitTests.java @@ -73,7 +73,7 @@ public class SelectBuilderUnitTests { Column foo = table.column("foo"); Comparison condition = foo.isEqualTo(SQL.literalOf("bar")); - Select select = builder.select(foo).from(table.getName()).where(condition).build(); + Select select = builder.select(foo).from(table).where(condition).build(); CapturingVisitor visitor = new CapturingVisitor(); select.visit(visitor); diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java index 1e183b57e..077d13458 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/SelectRendererUnitTests.java @@ -17,8 +17,10 @@ package org.springframework.data.relational.core.sql.render; import static org.assertj.core.api.Assertions.*; -import org.junit.Ignore; import org.junit.Test; + +import org.springframework.data.relational.core.dialect.PostgresDialect; +import org.springframework.data.relational.core.dialect.RenderContextFactory; import org.springframework.data.relational.core.sql.Column; import org.springframework.data.relational.core.sql.Conditions; import org.springframework.data.relational.core.sql.Expressions; @@ -26,6 +28,7 @@ import org.springframework.data.relational.core.sql.Functions; import org.springframework.data.relational.core.sql.OrderByField; import org.springframework.data.relational.core.sql.SQL; import org.springframework.data.relational.core.sql.Select; +import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.data.relational.core.sql.Table; import org.springframework.util.StringUtils; @@ -329,4 +332,21 @@ public class SelectRendererUnitTests { assertThat(rendered).isEqualTo("SELECT COUNT(foo.*) AS counter FROM foo"); } + + @Test // DATAJDBC-479 + public void shouldRenderWithRenderContext() { + + Table table = Table.create(SqlIdentifier.quoted("my_table")); + Table join_table = Table.create(SqlIdentifier.quoted("join_table")); + Select select = Select.builder() // + .select(Functions.count(table.asterisk()).as("counter"), table.column(SqlIdentifier.quoted("reserved_keyword"))) // + .from(table) // + .join(join_table).on(table.column("source")).equals(join_table.column("target")).build(); + + String rendered = SqlRenderer.create(new RenderContextFactory(PostgresDialect.INSTANCE).createRenderContext()) + .render(select); + + assertThat(rendered).isEqualTo( + "SELECT COUNT(\"my_table\".*) AS counter, \"my_table\".\"reserved_keyword\" FROM \"my_table\" JOIN \"join_table\" ON \"my_table\".source = \"join_table\".target"); + } }