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)
hibernate · Jan 1, 2023 · 689cca1 · 689cca1
1 parent 6da38d0
commit 689cca1
Show file tree

Hide file tree

Showing 13 changed files with 167 additions and 22 deletions.
diff --git 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 @@
 /**
  * Specifies that an Oracle-style {@code rowid} should be used in SQL
  * {@code update} statements for an entity, instead of the primary key.
+ * <p>
+ * 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 @@
 	 * Specifies the {@code rowid} identifier.
 	 * <p>
 	 * 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
@@ -215,6 +215,7 @@
 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 SchemaManagementTool getFallbackSchemaManagementTool(
 			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
@@ -75,6 +75,7 @@
 
 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 String getDisableConstraintsStatement() {
 	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
@@ -1406,4 +1406,9 @@ public UniqueDelegate getUniqueDelegate() {
 	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
@@ -1349,7 +1349,17 @@ public boolean canBatchTruncate() {
 	// 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/...e-core/src/main/java/org/hibernate/metamodel/mapping/internal/EntityRowIdMappingImpl.java b/...e-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 EntityRowIdMappingImpl(String rowIdName, String tableExpression, EntityMa
 		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
@@ -544,7 +544,13 @@ public 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
@@ -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 <X> ValueBinder<X> getBinder(JavaType<X> 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 <X> ValueExtractor<X> getExtractor(JavaType<X> 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
@@ -15,7 +15,6 @@
 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/...nate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java b/...nate-core/src/main/java/org/hibernate/type/descriptor/jdbc/internal/JdbcTypeBaseline.java
@@ -25,6 +25,7 @@
 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 static void prime(BaselineTarget target) {
 		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
@@ -19,7 +19,6 @@
  * <ul>
  *     <li>{@link java.sql.Types#DATALINK DATALINK}</li>
  *     <li>{@link java.sql.Types#DISTINCT DISTINCT}</li>
- *     <li>{@link java.sql.Types#ROWID ROWID}</li>
  *     <li>{@link java.sql.Types#REF REF}</li>
  *     <li>{@link java.sql.Types#REF_CURSOR REF_CURSOR}</li>
  * </ul>

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
@@ -13,11 +13,9 @@
 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 @@
  */
 @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 @@ void setUp(SessionFactoryScope scope) {
 	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<String> 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 @@ void testRowId(SessionFactoryScope scope) {
 			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,23 +85,23 @@ void testRowId(SessionFactoryScope scope) {
 
 	@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;
 
 		@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
@@ -17,8 +17,6 @@
 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 @@ public boolean isMatch(Dialect dialect) {
 			return dialect.supportsRecursiveCTE();
 		}
 	}
+
+	public static class SupportsRowId implements DialectCheck {
+		@Override
+		public boolean isMatch(Dialect dialect) {
+			return dialect.rowId() != null;
+		}
+	}
 }