From 689cca1963dd5ad01a008e59cdb227396b650961 Mon Sep 17 00:00:00 2001 From: Gavin Date: Sun, 1 Jan 2023 04:26:02 +0100 Subject: [PATCH] HHH-15958 much better support for @RowId annotation - the rowid pseudo-column and type are now determined automatically from Dialect - works (after all these years) in Postgres (and also on h2) - introduce RowIdJdbcType (not strictly necessary, but a nicety) --- .../java/org/hibernate/annotations/RowId.java | 9 ++- .../java/org/hibernate/dialect/Dialect.java | 20 +++++ .../java/org/hibernate/dialect/H2Dialect.java | 11 +++ .../org/hibernate/dialect/OracleDialect.java | 5 ++ .../hibernate/dialect/PostgreSQLDialect.java | 12 ++- .../internal/EntityRowIdMappingImpl.java | 8 +- .../entity/AbstractEntityPersister.java | 8 +- .../type/descriptor/jdbc/RowIdJdbcType.java | 77 +++++++++++++++++++ .../descriptor/jdbc/SmallIntJdbcType.java | 1 - .../jdbc/internal/JdbcTypeBaseline.java | 3 + .../type/descriptor/jdbc/package-info.java | 1 - .../hibernate/orm/test/rowid/RowIdTest.java | 25 +++--- .../org/hibernate/testing/DialectChecks.java | 9 ++- 13 files changed, 167 insertions(+), 22 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/RowIdJdbcType.java diff --git a/hibernate-core/src/main/java/org/hibernate/annotations/RowId.java b/hibernate-core/src/main/java/org/hibernate/annotations/RowId.java index e394126001..bbbcfb6389 100644 --- a/hibernate-core/src/main/java/org/hibernate/annotations/RowId.java +++ b/hibernate-core/src/main/java/org/hibernate/annotations/RowId.java @@ -15,6 +15,9 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; /** * Specifies that an Oracle-style {@code rowid} should be used in SQL * {@code update} statements for an entity, instead of the primary key. + *

+ * If the {@linkplain org.hibernate.dialect.Dialect SQL dialect} does + * not support some sort of {@code rowid}, this annotation is ignored. * * @author Steve Ebersole */ @@ -25,6 +28,10 @@ public @interface RowId { * Specifies the {@code rowid} identifier. *

* For example, on Oracle, this should be just {@code "rowid"}. + * + * @deprecated the {@code rowid} identifier is now inferred + * automatically from the {@link org.hibernate.dialect.Dialect} */ - String value(); + @Deprecated(since = "6.2") + String value() default ""; } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index 6fdbe48dae..47dfe43d52 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -215,6 +215,7 @@ import static org.hibernate.type.SqlTypes.NCLOB; import static org.hibernate.type.SqlTypes.NUMERIC; import static org.hibernate.type.SqlTypes.NVARCHAR; import static org.hibernate.type.SqlTypes.REAL; +import static org.hibernate.type.SqlTypes.ROWID; import static org.hibernate.type.SqlTypes.SMALLINT; import static org.hibernate.type.SqlTypes.TIME; import static org.hibernate.type.SqlTypes.TIMESTAMP; @@ -4622,4 +4623,23 @@ public abstract class Dialect implements ConversionContext { ServiceRegistryImplementor registry) { return new HibernateSchemaManagementTool(); } + + /** + * The name of a {@code rowid}-like pseudo-column which + * acts as a high-performance row locator, or null if + * this dialect has no such pseudo-column. + */ + public String rowId() { + return null; + } + + /** + * The JDBC type code of the {@code rowid}-like pseudo-column + * which acts as a high-performance row locator. + * + * @return {@link Types#ROWID} by default + */ + public int rowIdSqlType() { + return ROWID; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java index 7687dafbfe..0ad061f097 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java @@ -75,6 +75,7 @@ import jakarta.persistence.TemporalType; import static org.hibernate.query.sqm.TemporalUnit.SECOND; import static org.hibernate.type.SqlTypes.ARRAY; +import static org.hibernate.type.SqlTypes.BIGINT; import static org.hibernate.type.SqlTypes.BINARY; import static org.hibernate.type.SqlTypes.CHAR; import static org.hibernate.type.SqlTypes.DECIMAL; @@ -856,4 +857,14 @@ public class H2Dialect extends Dialect { public UniqueDelegate getUniqueDelegate() { return uniqueDelegate; } + + @Override + public String rowId() { + return "_rowid_"; + } + + @Override + public int rowIdSqlType() { + return BIGINT; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java index 689dac2dc4..a47a663d2b 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java @@ -1406,4 +1406,9 @@ public class OracleDialect extends Dialect { public String getCreateUserDefinedTypeKindString() { return "object"; } + + @Override + public String rowId() { + return "rowid"; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java index 03944f2f83..63cd7183b1 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java @@ -1349,7 +1349,17 @@ public class PostgreSQLDialect extends Dialect { // disabled foreign key constraints still prevent 'truncate table' // (these would help if we used 'delete' instead of 'truncate') -// @Override + @Override + public String rowId() { + return "ctid"; + } + + @Override + public int rowIdSqlType() { + return OTHER; + } + + // @Override // public String getDisableConstraintsStatement() { // return "set constraints all deferred"; // } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityRowIdMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityRowIdMappingImpl.java index 49aaeda8c1..e8c1899507 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityRowIdMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityRowIdMappingImpl.java @@ -6,9 +6,9 @@ */ package org.hibernate.metamodel.mapping.internal; -import java.sql.Types; import java.util.function.BiConsumer; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.mapping.IndexedConsumer; import org.hibernate.metamodel.mapping.EntityMappingType; @@ -41,9 +41,9 @@ public class EntityRowIdMappingImpl implements EntityRowIdMapping { this.rowIdName = rowIdName; this.tableExpression = tableExpression; this.declaringType = declaringType; - this.rowIdType = declaringType.getEntityPersister().getFactory().getTypeConfiguration() - .getBasicTypeRegistry() - .resolve( Object.class, Types.ROWID ); + final SessionFactoryImplementor factory = declaringType.getEntityPersister().getFactory(); + this.rowIdType = factory.getTypeConfiguration().getBasicTypeRegistry() + .resolve( Object.class, factory.getJdbcServices().getDialect().rowIdSqlType() ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 6e3481f5a3..69c1ab5461 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -544,7 +544,13 @@ public abstract class AbstractEntityPersister rootTableKeyColumnReaderTemplates = new String[identifierColumnSpan]; identifierAliases = new String[identifierColumnSpan]; - rowIdName = persistentClass.getRootTable().getRowId(); + final String rowId = persistentClass.getRootTable().getRowId(); + if ( rowId == null ) { + rowIdName = null; + } + else { + rowIdName = rowId.isEmpty() ? dialect.rowId() : rowId; + } if ( persistentClass.getLoaderName() != null ) { // We must resolve the named query on-demand through the boot model because it isn't initialized yet diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/RowIdJdbcType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/RowIdJdbcType.java new file mode 100644 index 0000000000..b11f3a6167 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/RowIdJdbcType.java @@ -0,0 +1,77 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.type.descriptor.jdbc; + +import org.hibernate.type.SqlTypes; +import org.hibernate.type.descriptor.ValueBinder; +import org.hibernate.type.descriptor.ValueExtractor; +import org.hibernate.type.descriptor.WrapperOptions; +import org.hibernate.type.descriptor.java.JavaType; + +import java.sql.CallableStatement; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.RowId; +import java.sql.SQLException; +import java.sql.Types; + +/** + * Descriptor for {@link Types#ROWID ROWID} handling. + * + * @author Gavin King + */ +public class RowIdJdbcType implements JdbcType { + public static final RowIdJdbcType INSTANCE = new RowIdJdbcType(); + + @Override + public int getJdbcTypeCode() { + return SqlTypes.ROWID; + } + + @Override + public String toString() { + return "RowIdJdbcType"; + } + + @Override + public ValueBinder getBinder(JavaType javaType) { + return new BasicBinder<>( javaType, this ) { + @Override + protected void doBind(PreparedStatement st, X value, int index, WrapperOptions options) + throws SQLException { + st.setRowId( index, getJavaType().unwrap( value, RowId.class, options ) ); + } + + @Override + protected void doBind(CallableStatement st, X value, String name, WrapperOptions options) + throws SQLException { + st.setRowId( name, getJavaType().unwrap( value, RowId.class, options ) ); + } + }; + } + + @Override + @SuppressWarnings("unchecked") + public ValueExtractor getExtractor(JavaType javaType) { + return new BasicExtractor<>( javaType, this ) { + @Override + protected X doExtract(ResultSet rs, int paramIndex, WrapperOptions options) throws SQLException { + return getJavaType().wrap( rs.getRowId( paramIndex ), options ); + } + + @Override + protected X doExtract(CallableStatement statement, int index, WrapperOptions options) throws SQLException { + return getJavaType().wrap( statement.getRowId( index ), options ); + } + + @Override + protected X doExtract(CallableStatement statement, String name, WrapperOptions options) throws SQLException { + return getJavaType().wrap( statement.getObject( name ), options ); + } + }; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/SmallIntJdbcType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/SmallIntJdbcType.java index a3eb6bcaed..9fef24f505 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/SmallIntJdbcType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/SmallIntJdbcType.java @@ -15,7 +15,6 @@ import java.sql.Types; import org.hibernate.type.descriptor.ValueBinder; import org.hibernate.type.descriptor.ValueExtractor; import org.hibernate.type.descriptor.WrapperOptions; -import org.hibernate.type.descriptor.java.BasicJavaType; import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.descriptor.jdbc.internal.JdbcLiteralFormatterNumericData; import org.hibernate.type.spi.TypeConfiguration; diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java index 2f50fc1bad..03a5b03eac 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java @@ -25,6 +25,7 @@ import org.hibernate.type.descriptor.jdbc.LongVarbinaryJdbcType; import org.hibernate.type.descriptor.jdbc.LongVarcharJdbcType; import org.hibernate.type.descriptor.jdbc.NumericJdbcType; import org.hibernate.type.descriptor.jdbc.RealJdbcType; +import org.hibernate.type.descriptor.jdbc.RowIdJdbcType; import org.hibernate.type.descriptor.jdbc.SmallIntJdbcType; import org.hibernate.type.descriptor.jdbc.TimeJdbcType; import org.hibernate.type.descriptor.jdbc.TimestampJdbcType; @@ -84,5 +85,7 @@ public class JdbcTypeBaseline { target.addDescriptor( Types.LONGNVARCHAR, LongVarcharJdbcType.INSTANCE ); target.addDescriptor( Types.NCLOB, ClobJdbcType.DEFAULT ); target.addDescriptor( new LongVarcharJdbcType(SqlTypes.LONG32NVARCHAR) ); + + target.addDescriptor( RowIdJdbcType.INSTANCE ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/package-info.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/package-info.java index 3aa14ca881..1a062627ca 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/package-info.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/package-info.java @@ -19,7 +19,6 @@ *

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/rowid/RowIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/rowid/RowIdTest.java index 404550eaa3..226128dad1 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/rowid/RowIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/rowid/RowIdTest.java @@ -13,11 +13,9 @@ import jakarta.persistence.Id; import jakarta.persistence.Table; import org.hibernate.annotations.RowId; -import org.hibernate.dialect.OracleDialect; import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; -import org.hibernate.testing.orm.junit.RequiresDialect; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.BeforeEach; @@ -33,14 +31,13 @@ import static org.junit.Assert.assertThat; */ @DomainModel( annotatedClasses = RowIdTest.Product.class ) @SessionFactory(statementInspectorClass = SQLStatementInspector.class) -@RequiresDialect( value = OracleDialect.class) public class RowIdTest { @BeforeEach void setUp(SessionFactoryScope scope) { scope.inTransaction( session -> { Product product = new Product(); - product.setId( 1L ); + product.setId( "1L" ); product.setName( "Mobile phone" ); product.setNumber( "123-456-7890" ); session.persist( product ); @@ -51,15 +48,19 @@ public class RowIdTest { void testRowId(SessionFactoryScope scope) { final String updatedName = "Smart phone"; scope.inTransaction( session -> { + String rowId = scope.getSessionFactory().getJdbcServices().getDialect().rowId(); + SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector(); statementInspector.clear(); - Product product = session.find( Product.class, 1L ); + Product product = session.find( Product.class, "1L" ); List sqls = statementInspector.getSqlQueries(); assertThat( sqls, hasSize( 1 ) ); - assertThat( sqls.get(0).matches( "(?i).*\\bselect\\b.+\\.ROWID.*\\bfrom\\s+product\\b.*" ), is( true ) ); + assertThat( rowId == null + || sqls.get(0).matches( "(?i).*\\bselect\\b.+\\." + rowId + ".*\\bfrom\\s+product\\b.*" ), + is( true ) ); assertThat( product.getName(), not( is( updatedName ) ) ); @@ -71,7 +72,9 @@ public class RowIdTest { sqls = statementInspector.getSqlQueries(); assertThat( sqls, hasSize( 1 ) ); - assertThat( sqls.get( 0 ).matches( "(?i).*\\bupdate\\s+product\\b.+?\\bwhere\\s+ROWID\\s*=.*" ), is( true ) ); + assertThat( rowId == null + || sqls.get( 0 ).matches( "(?i).*\\bupdate\\s+product\\b.+?\\bwhere\\s+" + rowId + "\\s*=.*" ), + is( true ) ); } ); scope.inTransaction( session -> { @@ -82,11 +85,11 @@ public class RowIdTest { @Entity(name = "Product") @Table(name = "product") - @RowId("ROWID") + @RowId public static class Product { @Id - private Long id; + private String id; @Column(name = "`name`") private String name; @@ -94,11 +97,11 @@ public class RowIdTest { @Column(name = "`number`") private String number; - public Long getId() { + public String getId() { return id; } - public void setId(Long id) { + public void setId(String id) { this.id = id; } diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/DialectChecks.java b/hibernate-testing/src/main/java/org/hibernate/testing/DialectChecks.java index f8a627d96c..4d33c6cbd5 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/DialectChecks.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/DialectChecks.java @@ -17,8 +17,6 @@ import org.hibernate.dialect.NationalizationSupport; import org.hibernate.dialect.PostgreSQLDialect; import org.hibernate.dialect.TiDBDialect; -import org.hibernate.testing.orm.junit.DialectFeatureCheck; - /** * Container class for different implementation of the {@link DialectCheck} interface. * @@ -312,4 +310,11 @@ abstract public class DialectChecks { return dialect.supportsRecursiveCTE(); } } + + public static class SupportsRowId implements DialectCheck { + @Override + public boolean isMatch(Dialect dialect) { + return dialect.rowId() != null; + } + } }