Avoid creation of multiple connections during schema migration #1928
Conversation
* Format code * Remove unnecessary `throws SqlClientException`
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| @Override | ||
| public BigDecimal getBigDecimal(int columnIndex, int scale) throws SQLException { | ||
| return delegate.getBigDecimal(columnIndex, scale); | ||
| return delegate.getBigDecimal( columnIndex, scale ); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| } | ||
|
|
||
| @Override | ||
| public InputStream getUnicodeStream(int columnIndex) throws SQLException { | ||
| return delegate.getUnicodeStream(columnIndex); | ||
| return delegate.getUnicodeStream( columnIndex ); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| } | ||
|
|
||
| @Override | ||
| public BigDecimal getBigDecimal(String columnLabel, int scale) throws SQLException { | ||
| return delegate.getBigDecimal(columnLabel, scale); | ||
| return delegate.getBigDecimal( columnLabel, scale ); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| } | ||
|
|
||
| @Override | ||
| public InputStream getUnicodeStream(String columnLabel) throws SQLException { | ||
| return delegate.getUnicodeStream(columnLabel); | ||
| return delegate.getUnicodeStream( columnLabel ); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| * | ||
| * @return the CompletionStage<ResultSet> from executing the query. | ||
| */ | ||
| public CompletionStage<ResultSet> selectJdbcOutsideTransaction(String sql, Object[] paramValues) { |
Check notice
Code scanning / CodeQL
Missing Override annotation Note
WDYM? |
|
I was referring to this comment: quarkusio/quarkus#39930 (comment) It made me think we don't actually need to do it. But now that I re-read it, it seems that we need to do it but it's a bit unclear why. Can you shed some light about this? |
|
I mean there are various reasons we might want to do that. For example, increment on a table-based id generator should happen outside the current tx. For DDL execution, well, IIRC that's something we've always done non-transactionally. I know there are some databases (Postgres, IIRC) which do support transactional DDL, but I doubt that this is something we need/want. |
|
Thanks |
Before we were creating a connection and then ignoring it for each query required to update the schema or collect metatada. Now the method for running queries outside the "current" transaction is in the SqlClientPool.
hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/ReactiveConnection.java
Show resolved
Hide resolved
...nate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/ExternalSqlClientPool.java
Show resolved
Hide resolved
...main/java/org/hibernate/reactive/provider/service/ReactiveImprovedExtractionContextImpl.java
Show resolved
Hide resolved
So maybe let's merge and improve later if necessary? |
|
Alright then, let's merge. Thanks @DavideD! |
Fix #1909
I'm not sure if it strictly necessary anymore to run the queries outside of the current transaction, but I think this way the code is simpler and consistent to what we were doing before. And the code is cleaner.
Basically, I moved the method we used to run the query in the pool.