Skip to content

Commit

Permalink
[#1792] Throw an exception if there is no result
Browse files Browse the repository at this point in the history
When running a native query that doesn't return any result (for example,
the creation of a stored procedure), `io.vertx.sqlclient.SqlResult#columnsNames` will be null.

This commit doesn two things:
1. add null check to avoid a NPE if `.columnsNames` returns null;
2. Throw an HibernateException if the user call getSingleResultOrNull
   but the query doesn't return any result.

   Note that we could return null in this situation, but we've decided
   to be consistent with Hibernate ORM, where the JDBC driver will throw
   the following exception (with PostgreSQL at least):
   ```
   Caused by: org.postgresql.util.PSQLException: No results were returned by the query.
	at org.postgresql.jdbc.PgPreparedStatement.executeQuery(PgPreparedStatement.java:135)
	at org.hibernate.sql.results.jdbc.internal.DeferredResultSetAccess.executeQuery(DeferredResultSetAccess.java:240)
   ```
  • Loading branch information
DavideD committed Nov 23, 2023
1 parent db4c3b5 commit 26d96e9
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.Calendar;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.function.Function;

import org.hibernate.engine.jdbc.BlobProxy;
Expand All @@ -42,6 +42,9 @@
import io.vertx.sqlclient.RowSet;
import io.vertx.sqlclient.desc.ColumnDescriptor;

import static java.util.Collections.emptyList;
import static java.util.Objects.requireNonNull;

/**
* An adaptor that allows Hibernate core code which expects a JDBC
* {@code ResultSet} to read values from Vert.x's {@code RowSet}.
Expand All @@ -54,6 +57,7 @@ public class ResultSetAdaptor implements ResultSet {
private boolean wasNull;

public ResultSetAdaptor(RowSet<Row> rows) {
requireNonNull( rows );
this.iterator = rows.iterator();
this.rows = rows;
}
Expand Down Expand Up @@ -199,15 +203,26 @@ public String getString(String columnLabel) {
}

private <T> T caseInsensitiveGet(String columnLabel, Function<String, T> produce) {
for ( String columnName : rows.columnsNames() ) {
for ( String columnName : getColumnsNames() ) {
if ( columnName.equalsIgnoreCase( columnLabel ) ) {
return produce.apply( columnName );
}
}

// Same error thrown by io.vertx.sqlclient.Row when it doesn't find the label
throw new NoSuchElementException( "Column " + columnLabel + " does not exist" );
}

/**
* rows.columnsNames() might return null for some databases.
* For example, when creating a stored procedure in PostgreSQL using a native query.
*
* @return A list of column names or an empty list.
*/
private List<String> getColumnsNames() {
return rows.columnsNames() == null ? emptyList() : rows.columnsNames();
}

@Override
public boolean getBoolean(String columnLabel) {
try {
Expand Down Expand Up @@ -368,7 +383,7 @@ public ResultSetMetaData getMetaData() {
return new ResultSetMetaData() {
@Override
public int getColumnCount() {
return rows.columnsNames().size();
return getColumnsNames() == null ? 0 : getColumnsNames().size();
}

@Override
Expand All @@ -379,12 +394,12 @@ public int getColumnType(int column) {

@Override
public String getColumnLabel(int column) {
return rows.columnsNames().get( column - 1 );
return getColumnsNames().get( column - 1 );
}

@Override
public String getColumnName(int column) {
return rows.columnsNames().get( column - 1 );
return getColumnsNames().get( column - 1 );
}

@Override
Expand Down Expand Up @@ -502,7 +517,7 @@ public Object getObject(String columnLabel) {
public int findColumn(String columnLabel) {
// JDBC parameters index start from 1
int index = 1;
for ( String column : rows.columnsNames() ) {
for ( String column : getColumnsNames() ) {
// Some dbs, like Oracle and Db2, return the column names always in uppercase
if ( column.equalsIgnoreCase( columnLabel ) ) {
return index;
Expand Down Expand Up @@ -865,7 +880,7 @@ private static class RowIdAdaptor implements RowId {
private final Buffer buffer;

private RowIdAdaptor(Buffer buffer) {
Objects.requireNonNull( buffer );
requireNonNull( buffer );
this.buffer = buffer;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.util.concurrent.CompletionException;
import java.util.concurrent.CompletionStage;
import java.util.function.Function;

Expand All @@ -24,6 +25,7 @@
import org.hibernate.reactive.pool.ReactiveConnection;
import org.hibernate.reactive.pool.impl.Parameters;
import org.hibernate.reactive.session.ReactiveConnectionSupplier;
import org.hibernate.reactive.util.impl.CompletionStages;
import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor;
import org.hibernate.sql.exec.spi.ExecutionContext;
import org.hibernate.sql.exec.spi.JdbcOperationQuerySelect;
Expand All @@ -41,7 +43,6 @@ public class ReactiveDeferredResultSetAccess extends DeferredResultSetAccess imp

private static final Log LOG = LoggerFactory.make( Log.class, MethodHandles.lookup() );
private final SqlStatementLogger sqlStatementLogger;
private final Function<String, PreparedStatement> statementCreator;

private final ExecutionContext executionContext;

Expand All @@ -59,7 +60,6 @@ public ReactiveDeferredResultSetAccess(
super( jdbcSelect, jdbcParameterBindings, executionContext, statementCreator );
this.executionContext = executionContext;
this.sqlStatementLogger = executionContext.getSession().getJdbcServices().getSqlStatementLogger();
this.statementCreator = statementCreator;
}

@Override
Expand Down Expand Up @@ -159,17 +159,33 @@ private CompletionStage<ResultSet> executeQuery() {
eventListenerManager.jdbcExecuteStatementStart();
return connection()
.selectJdbc( sql, parameters )
.thenCompose( this::validateResultSet )
.whenComplete( (resultSet, throwable) -> {
// FIXME: I don't know if this event makes sense for Vert.x
eventListenerManager.jdbcExecuteStatementEnd();
sqlStatementLogger.logSlowQuery( getFinalSql(), executeStartNanos );
} )
.thenCompose( this::reactiveSkipRows )
.handle( this::convertException );
.handle( CompletionStages::handle )
.thenCompose( handler -> handler.hasFailed()
? convertException( resultSet, handler.getThrowable() )
: handler.getResultAsCompletionStage()
);
} )
.whenComplete( (o, throwable) -> logicalConnection.afterStatement() );
}

private CompletionStage<ResultSet> validateResultSet(ResultSet resultSet) {
try {
return resultSet.getMetaData().getColumnCount() == 0
? failedFuture( LOG.noResultException( getFinalSql() ) )
: completedFuture( resultSet );
}
catch (SQLException e) {
throw new RuntimeException( e );
}
}

private ResultSet saveResultSet(ResultSet resultSet) {
this.resultSet = resultSet;
return saveColumnCount( resultSet );
Expand All @@ -185,18 +201,29 @@ private ReactiveConnection connection() {
return ( (ReactiveConnectionSupplier) executionContext.getSession() ).getReactiveConnection();
}

private ResultSet convertException(ResultSet resultSet, Throwable throwable) {
// FIXME: Vert.x will probably throw another exception. Check this.
if ( throwable instanceof SQLException) {
throw executionContext.getSession().getJdbcServices()
.getSqlExceptionHelper()
// FIXME: Add this to the logger?
.convert( (SQLException) throwable, "Exception executing SQL [" + getFinalSql() + "]" );
}
private <T> CompletionStage<T> convertException(T object, Throwable throwable) {
if ( throwable != null ) {
throw new HibernateException( throwable );
Throwable cause = throwable;
if ( throwable instanceof CompletionException ) {
cause = throwable.getCause();
}
// I doubt this is ever going to happen because Vert.x is not going to throw an SQLException
if ( cause instanceof SQLException ) {
return failedFuture( executionContext
.getSession().getJdbcServices()
.getSqlExceptionHelper()
.convert(
(SQLException) cause,
"Exception executing SQL [" + getFinalSql() + "]"
)
);
}
if ( cause instanceof HibernateException ) {
return failedFuture( cause );
}
return failedFuture( new HibernateException( cause ) );
}
return resultSet;
return completedFuture( object );
}

private CompletionStage<ResultSet> reactiveSkipRows(ResultSet resultSet) {
Expand Down

0 comments on commit 26d96e9

Please sign in to comment.