From 5ac8f1fb35eaf9b0adfd701c8f29fd968d3f91bb Mon Sep 17 00:00:00 2001 From: gmantele <gmantele@ari.uni-heidelberg.de> Date: Tue, 19 Apr 2016 18:31:51 +0200 Subject: [PATCH] [TAP] Finish fixing the transaction management bug. See the commit bd6218424df303974d8b151c55501c83233b8eff, for the first part of the fix (which actually did not really fixed the problem: connections "idle in transaction" were still in the database ; the connection being inside an opened transaction, it generates lock issues in the database in addition of probably taking some memory resources). --- src/tap/ADQLExecutor.java | 5 +- src/tap/config/ConfigurableTAPFactory.java | 4 +- src/tap/data/ResultSetTableIterator.java | 275 +++++++++++++-------- src/tap/db/DBConnection.java | 71 +++++- src/tap/db/JDBCConnection.java | 151 +++++++---- 5 files changed, 346 insertions(+), 160 deletions(-) diff --git a/src/tap/ADQLExecutor.java b/src/tap/ADQLExecutor.java index a16158a..fe7102a 100644 --- a/src/tap/ADQLExecutor.java +++ b/src/tap/ADQLExecutor.java @@ -417,11 +417,8 @@ public class ADQLExecutor { logger.logTAP(LogLevel.WARNING, report, "END_EXEC", "Can not drop the uploaded tables from the database!", e); } - // Free the connection (so that giving it back to a pool, if any, otherwise, just free resources): + // Free the connection (so that giving it back to a pool if any, otherwise just free resources): if (dbConn != null){ - // be sure no query is still processing and that any open transaction is rollbacked and ended: - dbConn.cancel(true); - // free the connection: service.getFactory().freeConnection(dbConn); dbConn = null; } diff --git a/src/tap/config/ConfigurableTAPFactory.java b/src/tap/config/ConfigurableTAPFactory.java index f337431..28b90f1 100644 --- a/src/tap/config/ConfigurableTAPFactory.java +++ b/src/tap/config/ConfigurableTAPFactory.java @@ -281,8 +281,8 @@ public class ConfigurableTAPFactory extends AbstractTAPFactory { @Override public void freeConnection(DBConnection conn){ try{ - // Cancel any possible query that could be running: - conn.cancel(true); + // End properly any query that is not yet stopped and cleaned (i.e. no more transaction opened): + conn.endQuery(); // Close the connection (if a connection pool is used, the connection is not really closed but is freed and kept in the pool for further usage): ((JDBCConnection)conn).getInnerConnection().close(); }catch(SQLException se){ diff --git a/src/tap/data/ResultSetTableIterator.java b/src/tap/data/ResultSetTableIterator.java index a2e60fb..c3ecf6f 100644 --- a/src/tap/data/ResultSetTableIterator.java +++ b/src/tap/data/ResultSetTableIterator.java @@ -22,7 +22,6 @@ package tap.data; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; -import java.sql.Statement; import java.sql.Timestamp; import java.util.NoSuchElementException; @@ -32,6 +31,7 @@ import adql.db.DBType.DBDatatype; import adql.db.STCS.Region; import adql.parser.ParseException; import adql.translator.JDBCTranslator; +import tap.db.DBConnection; import tap.metadata.TAPColumn; import uws.ISO8601Format; @@ -48,10 +48,10 @@ import uws.ISO8601Format; */ public class ResultSetTableIterator implements TableIterator { - /** Statement associated with the ResultSet/Dataset to read. + /** Connection associated with the ResultSet/Dataset to read. * <i>MAY be NULL</i> * @since 2.1 */ - private final Statement stmt; + private final DBConnection dbConn; /** ResultSet/Dataset to read. */ private final ResultSet data; @@ -84,10 +84,9 @@ public class ResultSetTableIterator implements TableIterator { * <h3>Type guessing</h3> * * <p> - * In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)} - * which deals with the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. - * This conversion is therefore not as precise as the one expected by a translator. That's why it is recommended - * to use one of the constructor having a {@link JDBCTranslator} in parameter. + * In order to guess a TAP type from a DBMS type, this constructor will call {@link #defaultTypeConversion(String, String[], String)} + * which proceeds a default conversion using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. + * This conversion is therefore not as precise as the one expected by the translator. * </p> * * @param dataSet Dataset over which this iterator must iterate. @@ -95,8 +94,7 @@ public class ResultSetTableIterator implements TableIterator { * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) */ public ResultSetTableIterator(final ResultSet dataSet) throws NullPointerException, DataReadException{ this(null, dataSet, null, null, null); @@ -106,31 +104,44 @@ public class ResultSetTableIterator implements TableIterator { * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p> * * <p> - * In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype - * from the DBMS column datatype (using {@link #convertType(int, String, String)}). + * In order to provide the metadata through {@link #getMetadata()}, this constructor is reading first the given metadata (if any), + * and then, try to guess the datatype from the DBMS column datatype (using {@link #convertType(int, String, String)}). * </p> * + * <h3>Provided metadata</h3> + * + * <p>The second parameter of this constructor aims to provide the metadata expected for each column of the ResultSet.</p> + * + * <p> + * For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn} + * instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept + * but the type will be guessed from the type provided by the ResultSetMetadata. + * </p> + * + * <p><i>Note: + * If this parameter is incomplete (array length less than the column count returned by the ResultSet or some array items are NULL), + * column metadata will be associated in the same order as the ResultSet columns. Missing metadata will be built from the + * {@link ResultSetMetaData} and so the types will be guessed. + * </i></p> + * * <h3>Type guessing</h3> * * <p> - * In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)} - * which deals with the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. - * This conversion is therefore not as precise as the one expected by a translator. That's why it is recommended - * to use one of the constructor having a {@link JDBCTranslator} in parameter. + * In order to guess a TAP type from a DBMS type, this constructor will call {@link #defaultTypeConversion(String, String[], String)} + * which proceeds a default conversion using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. + * This conversion is therefore not as precise as the one expected by the translator. * </p> * * @param dataSet Dataset over which this iterator must iterate. + * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. - * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. - * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @throws DataReadException If the metadata (columns count and types) can not be fetched. * - * @since 2.1 + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) */ - public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet) throws NullPointerException, DataReadException{ - this(stmt, dataSet, null, null, null); + public ResultSetTableIterator(final ResultSet dataSet, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{ + this(null, dataSet, resultMeta, null, null); } /** @@ -163,11 +174,14 @@ public class ResultSetTableIterator implements TableIterator { * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) + * + * @deprecated Use {@link #ResultSetTableIterator(ResultSet, JDBCTranslator, String)} instead ; using the translator without the DBMS name is generally not enough. + * It is then preferable to give also the DBMS name. */ + @Deprecated public ResultSetTableIterator(final ResultSet dataSet, final String dbms) throws NullPointerException, DataReadException{ - this(null, dataSet, null, dbms, null); + this(null, dataSet, null, null, dbms); } /** @@ -182,31 +196,27 @@ public class ResultSetTableIterator implements TableIterator { * * <p> * In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)} - * which deals with the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. - * This conversion is therefore not as precise as the one expected by a translator. That's why it is recommended - * to use one of the constructor having a {@link JDBCTranslator} in parameter. + * which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])}) + * if not NULL. However if no translator is provided, this function will proceed to a default conversion + * using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. + * This conversion is therefore not as precise as the one expected by the translator. * </p> * - * <p><i><b>Important</b>: - * The second parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. - * <b>This parameter is really used ONLY when the DBMS is SQLite ("sqlite").</b> - * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the - * ResultSet is coming. Without this information, type guessing will be unpredictable! - * </i></p> - * * @param dataSet Dataset over which this iterator must iterate. - * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> + * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert + * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> * * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) * - * @since 2.1 + * @deprecated Use {@link #ResultSetTableIterator(ResultSet, JDBCTranslator, String)} instead ; using the translator without the DBMS name is generally not enough. + * It is then preferable to give also the DBMS name. */ - public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final String dbms) throws NullPointerException, DataReadException{ - this(stmt, dataSet, null, dbms, null); + @Deprecated + public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator) throws NullPointerException, DataReadException{ + this(null, dataSet, null, translator, null); } /** @@ -227,28 +237,51 @@ public class ResultSetTableIterator implements TableIterator { * This conversion is therefore not as precise as the one expected by the translator. * </p> * + * <p><i><b>Important</b>: + * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. + * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> + * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the + * ResultSet is coming. Without this information, type guessing will be unpredictable! + * </i></p> + * * @param dataSet Dataset over which this iterator must iterate. * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> + * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) */ - public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator) throws NullPointerException, DataReadException{ - this(null, dataSet, translator, null, null); + public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ + this(null, dataSet, null, translator, dbms); } /** * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p> * * <p> - * In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype - * from the DBMS column datatype (using {@link #convertType(int, String, String)}). + * In order to provide the metadata through {@link #getMetadata()}, this constructor is reading first the given metadata (if any), + * and then, try to guess the datatype from the DBMS column datatype (using {@link #convertType(int, String, String)}). * </p> * + * <h3>Provided metadata</h3> + * + * <p>The third parameter of this constructor aims to provide the metadata expected for each column of the ResultSet.</p> + * + * <p> + * For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn} + * instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept + * but the type will be guessed from the type provide by the ResultSetMetadata. + * </p> + * + * <p><i>Note: + * If this parameter is incomplete (array length less than the column count returned by the ResultSet or some array items are NULL), + * column metadata will be associated in the same order as the ResultSet columns. Missing metadata will be built from the + * {@link ResultSetMetaData} and so the types will be guessed. + * </i></p> + * * <h3>Type guessing</h3> * * <p> @@ -259,30 +292,55 @@ public class ResultSetTableIterator implements TableIterator { * This conversion is therefore not as precise as the one expected by the translator. * </p> * + * <p><i><b>Important</b>: + * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. + * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> + * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the + * ResultSet is coming. Without this information, type guessing will be unpredictable! + * </i></p> + * * @param dataSet Dataset over which this iterator must iterate. * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> + * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> + * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. - * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. + * @throws DataReadException If the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) * - * @since 2.1 + * @deprecated Use {@link #ResultSetTableIterator(ResultSet, DBColumn[], JDBCTranslator, String)} instead ; only the position of the parameters has changed. */ - public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator) throws NullPointerException, DataReadException{ - this(stmt, dataSet, translator, null, null); + @Deprecated + public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{ + this(null, dataSet, resultMeta, translator, dbms); } /** * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p> * * <p> - * In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype - * from the DBMS column datatype (using {@link #convertType(int, String, String)}). + * In order to provide the metadata through {@link #getMetadata()}, this constructor is reading first the given metadata (if any), + * and then, try to guess the datatype from the DBMS column datatype (using {@link #convertType(int, String, String)}). * </p> * + * <h3>Provided metadata</h3> + * + * <p>The second parameter of this constructor aims to provide the metadata expected for each column of the ResultSet.</p> + * + * <p> + * For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn} + * instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept + * but the type will be guessed from the type provided by the ResultSetMetadata. + * </p> + * + * <p><i>Note: + * If this parameter is incomplete (array length less than the column count returned by the ResultSet or some array items are NULL), + * column metadata will be associated in the same order as the ResultSet columns. Missing metadata will be built from the + * {@link ResultSetMetaData} and so the types will be guessed. + * </i></p> + * * <h3>Type guessing</h3> * * <p> @@ -294,25 +352,25 @@ public class ResultSetTableIterator implements TableIterator { * </p> * * <p><i><b>Important</b>: - * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. + * The fourth parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the * ResultSet is coming. Without this information, type guessing will be unpredictable! * </i></p> * * @param dataSet Dataset over which this iterator must iterate. + * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. - * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. + * @throws DataReadException If the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) */ - public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ - this(null, dataSet, translator, dbms, null); + public ResultSetTableIterator(final ResultSet dataSet, final DBColumn[] resultMeta, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ + this(null, dataSet, resultMeta, translator, dbms); } /** @@ -326,43 +384,31 @@ public class ResultSetTableIterator implements TableIterator { * <h3>Type guessing</h3> * * <p> - * In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)} - * which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])}) - * if not NULL. However if no translator is provided, this function will proceed to a default conversion - * using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. + * In order to guess a TAP type from a DBMS type, this constructor will call {@link #defaultTypeConversion(String, String[], String)} + * which proceeds a default conversion using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. * This conversion is therefore not as precise as the one expected by the translator. * </p> * - * <p><i><b>Important</b>: - * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. - * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> - * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the - * ResultSet is coming. Without this information, type guessing will be unpredictable! - * </i></p> - * + * @param dbConn {@link DBConnection} instance which has provided the given result. * @param dataSet Dataset over which this iterator must iterate. - * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert - * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> - * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) * * @since 2.1 */ - public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ - this(stmt, dataSet, translator, dbms, null); + public ResultSetTableIterator(final DBConnection dbConn, final ResultSet dataSet) throws NullPointerException, DataReadException{ + this(dbConn, dataSet, null, null, null); } /** * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p> * * <p> - * In order to provide the metadata through {@link #getMetadata()}, this constructor is reading first the given metadata (if any), - * and then, try to guess the datatype from the DBMS column datatype (using {@link #convertType(int, String, String)}). + * In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype + * from the DBMS column datatype (using {@link #convertType(int, String, String)}). * </p> * * <h3>Provided metadata</h3> @@ -372,7 +418,7 @@ public class ResultSetTableIterator implements TableIterator { * <p> * For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn} * instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept - * but the type will be guessed from the type provide by the ResultSetMetadata. + * but the type will be guessed from the type provided by the ResultSetMetadata. * </p> * * <p><i>Note: @@ -384,6 +430,37 @@ public class ResultSetTableIterator implements TableIterator { * <h3>Type guessing</h3> * * <p> + * In order to guess a TAP type from a DBMS type, this constructor will call {@link #defaultTypeConversion(String, String[], String)} + * which proceeds a default conversion using the most common standard datatypes known in Postgres, SQLite, MySQL, Oracle and JavaDB/Derby. + * This conversion is therefore not as precise as the one expected by the translator. + * </p> + * + * @param dbConn {@link DBConnection} instance which has provided the given result. + * @param dataSet Dataset over which this iterator must iterate. + * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> + * + * @throws NullPointerException If NULL is given in parameter. + * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. + * + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) + * + * @since 2.1 + */ + public ResultSetTableIterator(final DBConnection dbConn, final ResultSet dataSet, final DBColumn[] metadata) throws NullPointerException, DataReadException{ + this(dbConn, dataSet, null, null, null); + } + + /** + * <p>Build a TableIterator able to read rows and columns of the given ResultSet.</p> + * + * <p> + * In order to provide the metadata through {@link #getMetadata()}, this constructor is trying to guess the datatype + * from the DBMS column datatype (using {@link #convertType(int, String, String)}). + * </p> + * + * <h3>Type guessing</h3> + * + * <p> * In order to guess a TAP type from a DBMS type, this constructor will call {@link #convertType(int, String, String)} * which will ask to the given translator ({@link JDBCTranslator#convertTypeFromDB(int, String, String, String[])}) * if not NULL. However if no translator is provided, this function will proceed to a default conversion @@ -392,26 +469,27 @@ public class ResultSetTableIterator implements TableIterator { * </p> * * <p><i><b>Important</b>: - * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. + * The fourth parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the * ResultSet is coming. Without this information, type guessing will be unpredictable! * </i></p> * + * @param dbConn {@link DBConnection} instance which has provided the given result. * @param dataSet Dataset over which this iterator must iterate. * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> - * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. - * @throws DataReadException If the metadata (columns count and types) can not be fetched. + * @throws DataReadException If the given ResultSet is closed or if the metadata (columns count and types) can not be fetched. * - * @see #convertType(int, String, String) - * @see #ResultSetTableIterator(Statement, ResultSet, JDBCTranslator, String, DBColumn[]) + * @see #ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) + * + * @since 2.1 */ - public ResultSetTableIterator(final ResultSet dataSet, final JDBCTranslator translator, final String dbms, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{ - this(null, dataSet, translator, dbms, resultMeta); + public ResultSetTableIterator(final DBConnection dbConn, final ResultSet dataSet, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ + this(dbConn, dataSet, null, translator, dbms); } /** @@ -429,7 +507,7 @@ public class ResultSetTableIterator implements TableIterator { * <p> * For that, it is expected that all these metadata are {@link TAPColumn} objects. Indeed, simple {@link DBColumn} * instances do not have the type information. If just {@link DBColumn}s are provided, the ADQL name it provides will be kept - * but the type will be guessed from the type provide by the ResultSetMetadata. + * but the type will be guessed from the type provided by the ResultSetMetadata. * </p> * * <p><i>Note: @@ -449,17 +527,18 @@ public class ResultSetTableIterator implements TableIterator { * </p> * * <p><i><b>Important</b>: - * The third parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. + * The fifth parameter of this constructor is given as second parameter of {@link #convertType(int, String, String)}. * <b>This parameter is really used ONLY when the translator conversion failed and when the DBMS is SQLite ("sqlite").</b> * Indeed, SQLite has so many datatype restrictions that it is absolutely needed to know it is the DBMS from which the * ResultSet is coming. Without this information, type guessing will be unpredictable! * </i></p> * + * @param dbConn {@link DBConnection} instance which has provided the given result. * @param dataSet Dataset over which this iterator must iterate. + * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * @param translator The {@link JDBCTranslator} used to transform the ADQL query into SQL query. This translator is also able to convert * JDBC types and to parse geometrical values. <i>note: MAY be NULL</i> * @param dbms Lower-case string which indicates from which DBMS the given ResultSet is coming. <i>note: MAY be NULL.</i> - * @param resultMeta List of expected columns. <i>note: these metadata are expected to be really {@link TAPColumn} objects ; MAY be NULL.</i> * * @throws NullPointerException If NULL is given in parameter. * @throws DataReadException If the metadata (columns count and types) can not be fetched. @@ -468,13 +547,13 @@ public class ResultSetTableIterator implements TableIterator { * * @since 2.1 */ - public ResultSetTableIterator(final Statement stmt, final ResultSet dataSet, final JDBCTranslator translator, final String dbms, final DBColumn[] resultMeta) throws NullPointerException, DataReadException{ + public ResultSetTableIterator(final DBConnection dbConn, final ResultSet dataSet, final DBColumn[] resultMeta, final JDBCTranslator translator, final String dbms) throws NullPointerException, DataReadException{ // A dataset MUST BE provided: if (dataSet == null) throw new NullPointerException("Missing ResultSet object over which to iterate!"); - // Set the associated statement: - this.stmt = stmt; + // Set the associated DBConnection: + this.dbConn = dbConn; // Keep a reference to the ResultSet: data = dataSet; @@ -514,13 +593,13 @@ public class ResultSetTableIterator implements TableIterator { try{ data.close(); rsClosed = true; - if (stmt != null) - stmt.close(); + if (dbConn != null) + dbConn.endQuery(); }catch(SQLException se){ if (!rsClosed) throw new DataReadException("Can not close the iterated ResultSet!", se); else - throw new DataReadException("ResultSet successfully closed but impossible to closed the associated Statement!", se); + throw new DataReadException("ResultSet successfully closed but impossible to end properly the query in the associated DBConnection!", se); } } diff --git a/src/tap/db/DBConnection.java b/src/tap/db/DBConnection.java index 612954e..af21338 100644 --- a/src/tap/db/DBConnection.java +++ b/src/tap/db/DBConnection.java @@ -16,17 +16,20 @@ package tap.db; * You should have received a copy of the GNU Lesser General Public License * along with TAPLibrary. If not, see <http://www.gnu.org/licenses/>. * - * Copyright 2012-2015 - UDS/Centre de DonnĂ©es astronomiques de Strasbourg (CDS), + * Copyright 2012-2016 - UDS/Centre de DonnĂ©es astronomiques de Strasbourg (CDS), * Astronomisches Rechen Institut (ARI) */ +import java.sql.ResultSet; +import java.sql.Statement; + +import adql.query.ADQLQuery; import tap.TAPFactory; import tap.data.DataReadException; import tap.data.TableIterator; import tap.metadata.TAPColumn; import tap.metadata.TAPMetadata; import tap.metadata.TAPTable; -import adql.query.ADQLQuery; /** * <p>Connection to the "database" (whatever is the type or whether it is linked to a true DBMS connection).</p> @@ -42,7 +45,7 @@ import adql.query.ADQLQuery; * </p> * * @author Grégory Mantelet (CDS;ARI) - * @version 2.1 (11/2015) + * @version 2.1 (04/2016) */ public interface DBConnection { @@ -107,7 +110,7 @@ public interface DBConnection { * * <h3>TAP_SCHEMA CREATION</h3> * <p> - * This function is MAY drop and then re-create the schema TAP_SCHEMA and all + * This function MAY drop and then re-create the schema TAP_SCHEMA and all * its tables listed in the TAP standard (TAP_SCHEMA.schemas, .tables, .columns, .keys and .key_columns). * <i>All other tables inside TAP_SCHEMA SHOULD NOT be modified!</i> * </p> @@ -115,7 +118,7 @@ public interface DBConnection { * <p> * The schema and the tables MUST be created using either the <b>standard definition</b> or the * <b>definition provided in the {@link TAPMetadata} object</b> (if provided). Indeed, if your definition of these TAP tables - * is different from the standard (the standard + new elements), you MUST provide your modifications in parameter + * is different from the standard (i.e. the standard + new elements), you MUST provide your modifications in parameter * through the {@link TAPMetadata} object so that they can be applied and taken into account in TAP_SCHEMA. * </p> * @@ -138,7 +141,7 @@ public interface DBConnection { * <li><b>(a) if TAP_SCHEMA tables are NOT provided</b>: * this function SHOULD consider their definition as exactly the one provided by * the TAP standard/protocol. If so, the standard definition MUST be automatically added - * into the {@link TAPMetadata} object AND into TAP_SCHEMA. + * into the {@link TAPMetadata} object AND into TAP_SCHEMA. * </li> * <li><b>(b) if TAP_SCHEMA tables ARE provided</b>: * the definition of all given elements will be taken into account while updating the TAP_SCHEMA. @@ -220,9 +223,18 @@ public interface DBConnection { * * <p>The result of this query must be formatted as a table, and so must be iterable using a {@link TableIterator}.</p> * - * <p><i>note: the interpretation of the ADQL query is up to the implementation. In most of the case, it is just needed + * <p><i>note: the interpretation of the ADQL query is up to the implementation. In most of the cases, it is just needed * to translate this ADQL query into an SQL query (understandable by the chosen DBMS).</i></p> * + * <p><b>IMPORTANT:</b> + * A {@link DBConnection} implementation may open resources to perform the query and get the result, + * but it may especially KEEP them OPENED in order to let the returned {@link TableIterator} iterates on + * the result set. So that closing these resources, the function {@link #endQuery()} should be called + * when the result is no longer needed. A good implementation of {@link TableIterator} SHOULD call this + * function when {@link TableIterator#close()} is called. <b>So, do not forget to call {@link TableIterator#close()} + * when you do not need any more the query result.</b> + * </p> + * * @param adqlQuery ADQL query to execute. * * @return The table result. @@ -230,6 +242,9 @@ public interface DBConnection { * @throws DBException If any errors occurs while executing the query. * * @since 2.0 + * + * @see #endQuery() + * @see TableIterator#close() */ public TableIterator executeQuery(final ADQLQuery adqlQuery) throws DBException; @@ -272,12 +287,14 @@ public interface DBConnection { * <p>Stop the execution of the current query.</p> * * <p> - * If asked. a rollback of the current transaction can also be performed - * after the cancellation (if successful) of the query. + * If asked, a rollback of the current transaction can also be performed + * before the function returns. This rollback operation is totally independent + * from the cancellation. It means that the rollback is always performed whatever + * is the cancellation result (or whatever the cancellation can be performed or not). * </p> * * <p> - * This function should <b>never</b> return any kind of exception. This is particularly important + * This function should <b>never</b> throw any kind of exception. This is particularly important * in the following cases: * </p> * <ul> @@ -286,7 +303,7 @@ public interface DBConnection { * <li>no query is currently running</li> * <li>a rollback is not possible or failed</li> * </ul> - * <p>However, if an exception occurs it may be directly logged at least as a WARNING.</p> + * <p>However, if an exception occurs it should be directly logged at least as a WARNING.</p> * * @param rollback <code>true</code> to cancel the statement AND rollback the current connection transaction, * <code>false</code> to just cancel the statement. @@ -295,4 +312,36 @@ public interface DBConnection { */ public void cancel(final boolean rollback); + /** + * <p>End the last query performed by this {@link DBConnection} and free some associated resources + * opened just for this last query.</p> + * + * <p> + * Originally, this function aims to be called when the result of {@link #executeQuery(ADQLQuery)} + * is no longer needed, in order to clean/free what the {@link DBConnection} needed to keep this + * result set open. In other words, if we take the example of a JDBC connection, this function will + * close the {@link ResultSet}, the {@link Statement} and will end any transaction eventually opened + * by the {@link DBConnection} (for instance if a fetchSize is set). + * </p> + * + * <p> + * However, this function could also be used after any other operation performed by the {@link DBConnection}. + * You should just be aware that, depending of the implementation, if a transaction has been opened, this + * function may end it, which means generally that a rollback is performed. + * </p> + * + * <p> + * Similarly, since it is supposed to end any query lastly performed, this function must also cancel + * any processing. So, the function {@link #cancel(boolean)} should be called. + * </p> + * + * <p> + * Finally, like {@link #cancel(boolean)}, this function should <b>never</b> throw any kind of exception. + * If internally an exception occurs, it should be directly logged at least as a WARNING. + * </p> + * + * @since 2.1 + */ + public void endQuery(); + } diff --git a/src/tap/db/JDBCConnection.java b/src/tap/db/JDBCConnection.java index 2a65ad6..d11c9b0 100644 --- a/src/tap/db/JDBCConnection.java +++ b/src/tap/db/JDBCConnection.java @@ -169,6 +169,13 @@ import uws.service.log.UWSLog.LogLevel; * To enable it, a simple call to {@link #setFetchSize(int)} is enough, whatever is the given value. * </i></p> * + * <p><i>Note 3: + * Generally set a fetch size starts a transaction in the database. So, after the result of the fetched query + * is not needed any more, do not forget to call {@link #endQuery()} in order to end the implicitly opened transaction. + * However, generally closing the returned {@link TableIterator} is fully enough (see the sources of + * {@link ResultSetTableIterator#close()} for more details). + * </i></p> + * * @author Grégory Mantelet (CDS;ARI) * @version 2.1 (04/2016) * @since 2.0 @@ -516,7 +523,7 @@ public class JDBCConnection implements DBConnection { * * <p><i>Note 1: * A failure of a rollback is not considered as a not supported cancellation feature by the JDBC driver or the DBMS. - * So if the cancellation succeeds but a rollback fails, a next call of this function will still try cancelling the given statement. + * So if the cancellation succeeds but a rollback fails, a next call of this function will still try canceling the given statement. * In case of a rollback failure, only a WARNING is written in the log file ; no exception is thrown. * </i></p> * @@ -541,13 +548,11 @@ public class JDBCConnection implements DBConnection { */ @Override public final void cancel(final boolean rollback){ - if (supportsCancel && stmt != null){ - synchronized(cancelled){ - cancelled = cancel(stmt, rollback); - // Log the success of the cancellation: - if (cancelled && logger != null) - logger.logDB(LogLevel.INFO, this, "CANCEL", "Query execution successfully stopped!", null); - } + synchronized(cancelled){ + cancelled = cancel(stmt, rollback); + // Log the success of the cancellation: + if (cancelled && logger != null) + logger.logDB(LogLevel.INFO, this, "CANCEL", "Query execution successfully stopped!", null); } } @@ -604,17 +609,8 @@ public class JDBCConnection implements DBConnection { } // Whatever happens, rollback all executed operations (only if rollback=true and if in a transaction ; that's to say if AutoCommit = false): finally{ - if (rollback && supportsTransaction){ - try{ - if (!connection.getAutoCommit()){ - connection.rollback(); - connection.setAutoCommit(true); - } - }catch(SQLException se){ - if (logger != null) - logger.logDB(LogLevel.ERROR, this, "CANCEL", "Query execution successfully stopped BUT the rollback fails!", se); - } - } + if (rollback && supportsTransaction) + rollback(); } } @@ -653,6 +649,18 @@ public class JDBCConnection implements DBConnection { } } + @Override + public void endQuery(){ + // Cancel the last query processing, if still running: + cancel(stmt, false); // note: this function is called instead of cancel(false) in order to avoid a log message about the cancellation operation result. + // Close the statement, if still opened: + closeStatement(); + // Rollback the transaction, if one has been opened: + rollback(false); + // End the transaction (i.e. go back to autocommit=true), if one has been opened: + endTransaction(false); + } + /* ********************* */ /* INTERROGATION METHODS */ /* ********************* */ @@ -709,13 +717,8 @@ public class JDBCConnection implements DBConnection { }catch(Exception ex){ // Close the ResultSet, if one was open: close(result); - // Ensure the query is not running anymore and rollback the transaction: - cancel(true); - // If still not canceled, log this fact as an error: - if (!isCancelled() && logger != null) - logger.logDB(LogLevel.ERROR, this, "EXECUTE", "Unexpected error while EXECUTING SQL query!", null); - // Close the open statement, if one was open: - closeStatement(); + // End properly the query: + endQuery(); // Propagate the exception with an appropriate error message: if (ex instanceof SQLException) throw new DBException("Unexpected error while executing a SQL query: " + ex.getMessage(), ex); @@ -731,12 +734,13 @@ public class JDBCConnection implements DBConnection { /** * <p>Create a {@link TableIterator} instance which lets reading the given result table.</p> * - * <p><b>Important note:</b> - * This function also set to NULL the statement of this {@link JDBCConnection} instance: {@link #stmt}. - * However, the statement is not closed ; it is just given to a {@link ResultSetTableIterator} iterator - * which will close it in the same time as the given {@link ResultSet}, when its function - * {@link ResultSetTableIterator#close()} is called. - * </p> + * <p><i>Note: + * The statement currently opened is not closed by this function. Actually, it is still associated with + * this {@link JDBCConnection}. However, this latter is provided to the {@link TableIterator} returned by + * this function. Thus, when the {@link TableIterator#close()} is called, the function {@link #endQuery()} + * will be called. It will then close the {@link ResultSet}, the {@link Statement} and end any opened + * transaction (with rollback). See {@link #endQuery()} for more details. + * </i></p> * * @param rs Result of an SQL query. * @param resultingColumns Metadata corresponding to each columns of the result. @@ -745,14 +749,12 @@ public class JDBCConnection implements DBConnection { * * @throws DataReadException If the metadata (columns count and types) can not be fetched * or if any other error occurs. + * + * @see ResultSetTableIterator#ResultSetTableIterator(DBConnection, ResultSet, DBColumn[], JDBCTranslator, String) */ protected TableIterator createTableIterator(final ResultSet rs, final DBColumn[] resultingColumns) throws DataReadException{ - // Dis-associate the current Statement from this JDBCConnection instance: - Statement itStmt = stmt; - stmt = null; - // Return a TableIterator wrapping the given ResultSet: try{ - return new ResultSetTableIterator(itStmt, rs, translator, dbms, resultingColumns); + return new ResultSetTableIterator(this, rs, resultingColumns, translator, dbms); }catch(Throwable t){ throw (t instanceof DataReadException) ? (DataReadException)t : new DataReadException(t); } @@ -852,7 +854,7 @@ public class JDBCConnection implements DBConnection { logger.logDB(LogLevel.ERROR, this, "LOAD_TAP_SCHEMA", "Impossible to create a Statement!", se); throw new DBException("Can not create a Statement!", se); }finally{ - cancel(true); + cancel(stmt, true); // note: this function is called instead of cancel(true) in order to avoid a log message about the cancellation operation result. closeStatement(); } @@ -2423,6 +2425,31 @@ public class JDBCConnection implements DBConnection { } } + /** + * <p>Rollback the current transaction. + * The success or the failure of the rollback operation is always logged (except if no logger is available).</p> + * + * <p> + * {@link #startTransaction()} must have been called before. If it's not the case the connection + * may throw a {@link SQLException} which will be transformed into a {@link DBException} here. + * </p> + * + * <p>If transactions are not supported by this connection, nothing is done.</p> + * + * <p><b><i>Important note:</b> + * If any error interrupts the ROLLBACK operation, transactions will considered afterwards as not supported by this connection. + * So, subsequent call to this function (and any other transaction related function) will never do anything. + * </i></p> + * + * @throws DBException If it is impossible to rollback a transaction though transactions are supported by this connection.. + * If these are not supported, this error can never be thrown. + * + * @see #rollback(boolean) + */ + protected final void rollback(){ + rollback(true); + } + /** * <p>Rollback the current transaction.</p> * @@ -2438,23 +2465,52 @@ public class JDBCConnection implements DBConnection { * So, subsequent call to this function (and any other transaction related function) will never do anything. * </i></p> * + * @param log <code>true</code> to log the success/failure of the rollback operation, + * <code>false</code> to be quiet whatever happens. + * * @throws DBException If it is impossible to rollback a transaction though transactions are supported by this connection.. * If these are not supported, this error can never be thrown. + * + * @since 2.1 */ - protected void rollback(){ + protected void rollback(final boolean log){ try{ - if (supportsTransaction){ + if (supportsTransaction && !connection.getAutoCommit()){ connection.rollback(); - if (logger != null) + if (log && logger != null) logger.logDB(LogLevel.INFO, this, "ROLLBACK", "Transaction ROLLBACKED.", null); } }catch(SQLException se){ supportsTransaction = false; - if (logger != null) + if (log && logger != null) logger.logDB(LogLevel.ERROR, this, "ROLLBACK", "Transaction ROLLBACK impossible!", se); } } + /** + * <p>End the current transaction. + * The success or the failure of the transaction ending operation is always logged (except if no logger is available).</p> + * + * <p> + * Basically, if transactions are supported by this connection, the flag AutoCommit is just turned on. + * </p> + * + * <p>If transactions are not supported by this connection, nothing is done.</p> + * + * <p><b><i>Important note:</b> + * If any error interrupts the END TRANSACTION operation, transactions will be afterwards considered as not supported by this connection. + * So, subsequent call to this function (and any other transaction related function) will never do anything. + * </i></p> + * + * @throws DBException If it is impossible to end a transaction though transactions are supported by this connection. + * If these are not supported, this error can never be thrown. + * + * @see #endTransaction(boolean) + */ + protected final void endTransaction(){ + endTransaction(true); + } + /** * <p>End the current transaction.</p> * @@ -2469,19 +2525,24 @@ public class JDBCConnection implements DBConnection { * So, subsequent call to this function (and any other transaction related function) will never do anything. * </i></p> * + * @param log <code>true</code> to log the success/failure of the transaction ending operation, + * <code>false</code> to be quiet whatever happens. + * * @throws DBException If it is impossible to end a transaction though transactions are supported by this connection. * If these are not supported, this error can never be thrown. + * + * @since 2.1 */ - protected void endTransaction(){ + protected void endTransaction(final boolean log){ try{ if (supportsTransaction){ connection.setAutoCommit(true); - if (logger != null) + if (log && logger != null) logger.logDB(LogLevel.INFO, this, "END_TRANSACTION", "Transaction ENDED.", null); } }catch(SQLException se){ supportsTransaction = false; - if (logger != null) + if (log && logger != null) logger.logDB(LogLevel.ERROR, this, "END_TRANSACTION", "Transaction ENDing impossible!", se); } } -- GitLab