Skip to content

[536] [913] [910] SchemaMigrator/SchemaValidator support for PostgreSQL and CockroachDB #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 31, 2021
Merged

[536] [913] [910] SchemaMigrator/SchemaValidator support for PostgreSQL and CockroachDB #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/tracking-orm-5.build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
strategy:
matrix:
example: [ 'session-example', 'native-sql-example' ]
orm-version: [ '[5.4,5.5)','[5.5,5.6)' ]
orm-version: [ '5.6.0.Beta1' ]
db: ['MySQL', 'PostgreSQL']
exclude:
# 'native-sql-example' doesn't run on MySQL because it has native queries
Expand Down Expand Up @@ -77,7 +77,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
orm-version: [ '[5.4,5.5)','[5.5,5.6)' ]
orm-version: [ '5.6.0.Beta1' ]
db: ['MariaDB', 'MySQL', 'PostgreSQL', 'DB2', 'CockroachDB', 'MSSQLServer']
steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ org.gradle.java.installations.auto-download=false
#enableMavenLocalRepo = true

# Override default Hibernate ORM version
#hibernateOrmVersion = 5.5.2.Final
#hibernateOrmVersion = 5.5.6-SNAPSHOT

# If set to true, skip Hibernate ORM version parsing (default is true, if set to null)
# this is required when using intervals or weird versions or the build will fail
Expand Down
57 changes: 40 additions & 17 deletions ...ate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,55 +74,55 @@ public boolean wasNull() {

@Override
public String getString(int columnIndex) {
String string = row.getString(columnIndex);
String string = row.getString( columnIndex - 1 );
return (wasNull=string==null) ? null : string;
}

@Override
public boolean getBoolean(int columnIndex) {
Boolean bool = row.getBoolean(columnIndex);
Boolean bool = row.getBoolean(columnIndex - 1);
wasNull = bool == null;
return !wasNull && bool;
}

@Override
public byte getByte(int columnIndex) {
Integer integer = row.getInteger( columnIndex );
Integer integer = row.getInteger( columnIndex - 1 );
wasNull = integer == null;
return wasNull ? 0 : integer.byteValue();
}

@Override
public short getShort(int columnIndex) {
Short aShort = row.getShort( columnIndex );
Short aShort = row.getShort( columnIndex - 1 );
wasNull = aShort == null;
return wasNull ? 0 : aShort;
}

@Override
public int getInt(int columnIndex) {
Integer integer = row.getInteger( columnIndex );
Integer integer = row.getInteger( columnIndex - 1 );
wasNull = integer == null;
return wasNull ? 0 : integer;
}

@Override
public long getLong(int columnIndex) {
Long aLong = row.getLong(columnIndex);
Long aLong = row.getLong( columnIndex - 1 );
wasNull = aLong == null;
return wasNull ? 0 : aLong;
}

@Override
public float getFloat(int columnIndex) {
Float real = row.getFloat( columnIndex );
Float real = row.getFloat( columnIndex - 1 );
wasNull = real == null;
return wasNull ? 0 : real;
}

@Override
public double getDouble(int columnIndex) {
Double real = row.getDouble( columnIndex );
Double real = row.getDouble( columnIndex - 1 );
wasNull = real == null;
return wasNull ? 0 : real;
}
Expand All @@ -134,32 +134,32 @@ public BigDecimal getBigDecimal(int columnIndex, int scale) {

@Override
public byte[] getBytes(int columnIndex) {
Buffer buffer = row.getBuffer(columnIndex);
Buffer buffer = row.getBuffer( columnIndex - 1 );
wasNull = buffer == null;
return wasNull ? null : buffer.getBytes();
}

@Override
public Date getDate(int columnIndex) {
LocalDate localDate = row.getLocalDate(columnIndex);
LocalDate localDate = row.getLocalDate( columnIndex - 1 );
return (wasNull=localDate==null) ? null : java.sql.Date.valueOf(localDate);
}

@Override
public Time getTime(int columnIndex) {
LocalTime localTime = row.getLocalTime(columnIndex);
LocalTime localTime = row.getLocalTime( columnIndex - 1);
return (wasNull=localTime==null) ? null : Time.valueOf(localTime);
}

@Override
public Time getTime(int columnIndex, Calendar cal) {
LocalTime localTime = row.getLocalTime(columnIndex);
LocalTime localTime = row.getLocalTime( columnIndex - 1 );
return (wasNull=localTime==null) ? null : Time.valueOf(localTime);
}

@Override
public Timestamp getTimestamp(int columnIndex) {
LocalDateTime localDateTime = row.getLocalDateTime(columnIndex);
LocalDateTime localDateTime = row.getLocalDateTime( columnIndex - 1 );
return (wasNull=localDateTime==null) ? null : Timestamp.valueOf(localDateTime);
}

Expand Down Expand Up @@ -214,7 +214,28 @@ public int getInt(String columnLabel) {

@Override
public long getLong(String columnLabel) {
Long aLong = row.getLong( columnLabel );
// PostgreSQL stores sequence metadata in information_schema as Strings.
// First try to get the value as a Long; if that fails, try to get
// as a String and try to parse it as a Long.
Long aLong;
try {
aLong = row.getLong(columnLabel);
}
catch (ClassCastException ex) {
// Check if the value is a String that can be converted to a Long
final String aString = row.getString(columnLabel);
// aString won't be null; check just because...
try {
aLong = aString != null ? Long.parseLong( aString ) : null;
}
catch (ClassCastException exNotAString) {
// The value is neither a long nor a String that can be
// parsed as a long.
// Throw the original exception.
throw ex;
}
}

wasNull = aLong == null;
return wasNull ? 0 : aLong;
}
Expand Down Expand Up @@ -311,7 +332,7 @@ public boolean isClosed() {

@Override
public <T> T getObject(int columnIndex, Class<T> type) {
T object = row.get(type, columnIndex);
T object = row.get(type, columnIndex - 1);
return (wasNull=object==null) ? null : object;
}

Expand Down Expand Up @@ -430,6 +451,8 @@ public String getCatalogName(int column) {

@Override
public String getColumnTypeName(int column) {
// This information is in rows.columnDescriptors().get( column-1 ).dataType.name
// but does not appear to be accessible.
return null;
}

Expand Down Expand Up @@ -467,7 +490,7 @@ public boolean isWrapperFor(Class<?> iface) {

@Override
public Object getObject(int columnIndex) {
Object object = row.getValue( columnIndex );
Object object = row.getValue( columnIndex - 1 );
return (wasNull=object==null) ? null : object;
}

Expand All @@ -485,7 +508,7 @@ public int findColumn(String columnLabel) {

@Override
public BigDecimal getBigDecimal(int columnIndex) {
BigDecimal decimal = row.getBigDecimal(columnIndex);
BigDecimal decimal = row.getBigDecimal(columnIndex - 1);
return (wasNull=decimal==null) ? null : decimal;
}

Expand Down
5 changes: 5 additions & 0 deletions hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/BatchingConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ public CompletionStage<ResultSet> selectJdbc(String sql, Object[] paramValues) {
delegate.selectJdbc(sql, paramValues);
}

@Override
public CompletionStage<ResultSet> selectJdbcOutsideTransaction(String sql, Object[] paramValues) {
return delegate.selectJdbcOutsideTransaction( sql, paramValues );
}

public CompletionStage<Long> selectIdentifier(String sql, Object[] paramValues) {
// Do not want to execute the batch here
// because we want to be able to select
Expand Down
16 changes: 16 additions & 0 deletions hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/ReactiveConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,22 @@ interface Expectation {
CompletionStage<Result> select(String sql, Object[] paramValues);
CompletionStage<ResultSet> selectJdbc(String sql, Object[] paramValues);

/**
* This method is intended to be used only for queries returning
* a ResultSet that must be executed outside of any "current"
* transaction (i.e with autocommit=true).
* <p/>
* For example, it would be appropriate to use this method when
* performing queries on information_schema or system tables in
* order to obtain metadata information about catalogs, schemas,
* tables, etc.
*
* @param sql - the query to execute outside of a transaction
* @param paramValues - a non-null array of parameter values
* @return the CompletionStage<ResultSet> from executing the query.
*/
CompletionStage<ResultSet> selectJdbcOutsideTransaction(String sql, Object[] paramValues);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remind us why you need the additional method?

Copy link
Contributor Author

@gbadner gbadner Jul 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavideD , since SchemaMigrator executes select queries on system tables and executes DDL statements (which update system tables), I assumed that these queries should be executed outside of a transaction (i.e., with autocommit=true). IIUC, this would help avoid contention with other work the DB is doing on schemas used by various applications.

If you look at the corresponding ORM code in GenerationTargetToDatabase, you can see that the statement is created using a connection obtained from a DdlTransactionIsolator.

Javadoc for DdlTransactionIsolator says:

 * Provides access to a Connection that is isolated from
 * any "current transaction" with the designed purpose of
 * performing DDL commands

If you look at the DdlTransactionIsolator implementations, you will see that autocommit is set to true on the Connection that is obtained:
DdlTransactionIsolatorJtaImpl
DdlTransactionIsolatorNonJtaImpl
DdlTransactionIsolatorProvidedConnectionImpl relies on JdbcConnectionAccessProvidedConnectionImpl which sets autocommit to true

I didn't see any other way to execute a query outside of a transaction that returned a CompletionStage<ResultSet>, so I created a new method.

Is there some other method that should be used instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we had this requirement before. I don't see any problem adding a new method.

But I mainly wanted a bit of an explanation written somewhere so that others know as well (maybe it wasn't obvious only to me though ;-)

At some point, we will need to add some javadoc to that class. Anyway, not something you need to worry about at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know this stuff is lacking in documentation. I will be adding that.


CompletionStage<Long> insertAndSelectIdentifier(String sql, Object[] paramValues);
CompletionStage<Long> selectIdentifier(String sql, Object[] paramValues);

Expand Down
5 changes: 5 additions & 0 deletions hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/ProxyConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ public CompletionStage<ResultSet> selectJdbc(String sql, Object[] paramValues) {
return withConnection( conn -> conn.selectJdbc( sql, paramValues ) );
}

@Override
public CompletionStage<ResultSet> selectJdbcOutsideTransaction(String sql, Object[] paramValues) {
return withConnection( conn -> conn.selectJdbcOutsideTransaction( sql, paramValues ) );
}

@Override
public CompletionStage<Long> selectIdentifier(String sql, Object[] paramValues) {
return withConnection( conn -> conn.selectIdentifier( sql, paramValues ) );
Expand Down
10 changes: 10 additions & 0 deletions ...ate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/SqlClientConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ public CompletionStage<ResultSet> selectJdbc(String sql, Object[] paramValues) {
return preparedQuery( sql, Tuple.wrap( paramValues ) ).thenApply(ResultSetAdaptor::new);
}

@Override
public CompletionStage<ResultSet> selectJdbcOutsideTransaction(String sql, Object[] paramValues) {
return preparedQueryOutsideTransaction( sql, Tuple.wrap( paramValues ) ).thenApply(ResultSetAdaptor::new);
}

@Override
public CompletionStage<Void> execute(String sql) {
return preparedQuery( sql ).thenApply( ignore -> null );
Expand Down Expand Up @@ -192,6 +197,11 @@ public CompletionStage<RowSet<Row>> preparedQueryOutsideTransaction(String sql)
return pool.preparedQuery( sql ).execute().toCompletionStage();
}

public CompletionStage<RowSet<Row>> preparedQueryOutsideTransaction(String sql, Tuple parameters) {
feedback( sql );
return pool.preparedQuery( sql ).execute( parameters ).toCompletionStage();
}

private void feedback(String sql) {
Objects.requireNonNull(sql, "SQL query cannot be null");
// DDL already gets formatted by the client, so don't reformat it
Expand Down
6 changes: 4 additions & 2 deletions ...ve-core/src/main/java/org/hibernate/reactive/provider/impl/ReactiveServiceInitiators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.hibernate.reactive.provider.service.NoJdbcEnvironmentInitiator;
import org.hibernate.reactive.provider.service.NoJtaPlatformInitiator;
import org.hibernate.reactive.provider.service.ReactiveQueryTranslatorFactoryInitiator;
import org.hibernate.reactive.provider.service.ReactiveSchemaManagementToolInitiator;
import org.hibernate.reactive.provider.service.ReactiveSessionFactoryBuilderInitiator;
import org.hibernate.reactive.id.impl.ReactiveIdentifierGeneratorFactoryInitiator;
import org.hibernate.reactive.provider.service.ReactivePersisterClassResolverInitiator;
Expand All @@ -38,7 +39,6 @@
import org.hibernate.resource.transaction.internal.TransactionCoordinatorBuilderInitiator;
import org.hibernate.service.internal.SessionFactoryServiceRegistryFactoryInitiator;
import org.hibernate.tool.hbm2ddl.ImportSqlCommandExtractorInitiator;
import org.hibernate.tool.schema.internal.SchemaManagementToolInitiator;

import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -83,7 +83,6 @@ private static List<StandardServiceInitiator> buildInitialServiceInitiatorList()
serviceInitiators.add( PropertyAccessStrategyResolverInitiator.INSTANCE );

serviceInitiators.add( ImportSqlCommandExtractorInitiator.INSTANCE );
serviceInitiators.add( SchemaManagementToolInitiator.INSTANCE );

//Custom for Hibernate Reactive:
serviceInitiators.add( NoJdbcEnvironmentInitiator.INSTANCE );
Expand All @@ -105,6 +104,9 @@ private static List<StandardServiceInitiator> buildInitialServiceInitiatorList()
serviceInitiators.add( JdbcServicesInitiator.INSTANCE );
serviceInitiators.add( RefCursorSupportInitiator.INSTANCE );

//Custom for Hibernate Reactive:
serviceInitiators.add( ReactiveSchemaManagementToolInitiator.INSTANCE );

//Custom for Hibernate Reactive:
serviceInitiators.add( ReactiveQueryTranslatorFactoryInitiator.INSTANCE );

Expand Down
Loading