From fa195b26d451503889c5c41c7b5e4d8c55a48109 Mon Sep 17 00:00:00 2001 From: Zac Spitzer Date: Tue, 16 Jul 2024 10:13:20 +0200 Subject: [PATCH 1/2] LDEV-5023 test case for qoq regression https://luceeserver.atlassian.net/browse/LDEV-5023 --- .../java/lucee/runtime/db/HSQLDBHandler.java | 24 ++-- test/tickets/LDEV5023.cfc | 105 ++++++++++++++++++ 2 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 test/tickets/LDEV5023.cfc diff --git a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java b/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java index afaf7cc9ea..2c29a5b927 100644 --- a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java +++ b/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java @@ -61,7 +61,7 @@ import lucee.runtime.type.StructImpl; import lucee.runtime.type.dt.TimeSpan; import lucee.runtime.type.util.CollectionUtil; - +import lucee.aprint; /** * class to reexecute queries on the resultset object inside the cfml environment */ @@ -374,6 +374,8 @@ private static Struct getUsedColumnsForQuery(Connection conn, SQL sql) { // TODO consider if worth doing, if recordcount / column count is too small + aprint.o(sql.toString()); + try { Statement stat = conn.createStatement(); stat.execute("CREATE VIEW " + view + " AS " + sql.toString()); // + StringUtil.toUpperCase(sql.toString())); @@ -403,8 +405,8 @@ private static Struct getUsedColumnsForQuery(Connection conn, SQL sql) { Struct tableCols = ((Struct) tables.get(tableName)); tableCols.setEL(Caster.toKey(rs.getString(colPos)), null); } - // aprint.o(rs); - // aprint.o(tables); + aprint.o(rs); + aprint.o(tables); // don't need the view anymore, bye bye stat.execute("DROP VIEW " + view); } @@ -569,19 +571,27 @@ public static QueryImpl __execute(PageContext pc, SQL sql, int maxrows, int fetc try { // we now only lock the data loading, not the execution of the query, but should this be done via // cflock by the developer? + aprint.out(tables); synchronized (lock) { Iterator it = tables.iterator(); String cfQueryName = null; // name of the source query variable String dbTableName = null; // name of the table in the database String modSql = null; // int len=tables.size(); + SystemOut.print("QoQ HSQLDB CREATED TABLES: " + sql.toString()); while (it.hasNext()) { cfQueryName = it.next().toString();// tables.get(i).toString(); dbTableName = cfQueryName.replace('.', '_'); + if (qoqTables.contains(dbTableName)){ + aprint.o("duplicate table name!!"); + } - // this could match the wrong strings?? - modSql = StringUtil.replace(sql.getSQLString(), cfQueryName, dbTableName, false); - sql.setSQLString(modSql); + if (!cfQueryName.toLowerCase().equals(dbTableName.toLowerCase())){ + // TODO this could match the wrong strings?? + modSql = StringUtil.replace(sql.getSQLString(), cfQueryName, dbTableName, false); + sql.setSQLString(modSql); + SystemOut.print("QoQ HSQLDB CREATED TABLES: " + modSql); + } if (sql.getItems() != null && sql.getItems().length > 0) sql = new SQLImpl(sql.toString()); // temp tables still get created will all the source columns, // only populateTables is driven by the required columns calculated from the view @@ -589,7 +599,7 @@ public static QueryImpl __execute(PageContext pc, SQL sql, int maxrows, int fetc qoqTables.add(dbTableName); } - // SystemOut.print("QoQ HSQLDB CREATED TABLES: " + sql.toString()); + // create the sql as a view, to find out which table columns are needed Struct allTableColumns = getUsedColumnsForQuery(conn, sql); diff --git a/test/tickets/LDEV5023.cfc b/test/tickets/LDEV5023.cfc new file mode 100644 index 0000000000..043eaf74dd --- /dev/null +++ b/test/tickets/LDEV5023.cfc @@ -0,0 +1,105 @@ +component extends="org.lucee.cfml.test.LuceeTestCase" labels="qoq" { + + function run( testResults , testBox ) { + + describe( title='QofQ' , body=function(){ + + it( title='QoQ select * from table same source table name HSQLDB', body=function() { + var q = extensionList(); + var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word + // native engine + cols = "name, id"; + systemOutput("native [#cols#]", true); + var q_native = QueryExecute( + sql = "SELECT #cols# FROM q", + options = { dbtype: 'query', maxrows=5 } + ); + var q_stash = duplicate( q_native ); + // hsqldb engine, coz join + systemOutput("hsqldb", true); + var q_hsqlb = QueryExecute( + sql = "SELECT t1.name FROM q_native t1, q_native t2 WHERE t1.id = t2.id", + options = { dbtype: 'query' } + ); + systemOutput( q_hsqlb, true ); + expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_stash.recordcount ).toBe( q_native.recordcount ); + }); + + it( title='QoQ select * from table same source table name (arguments) HSQLDB', body=function() { + var q = extensionList(); + var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word + // native engine + cols = "name, id"; + systemOutput("native [#cols#]", true); + var q_native = QueryExecute( + sql = "SELECT #cols# FROM q", + options = { dbtype: 'query', maxrows=5 } + ); + var q_stash = duplicate( q_native ); + // hsqldb engine, coz join + systemOutput("hsqldb", true); + arguments.q_native = q_native; + var q_hsqlb = QueryExecute( + sql = "SELECT t1.name FROM q_native t1, arguments.q_native t2 WHERE t1.id = t2.id", + options = { dbtype: 'query' } + ); + systemOutput( q_hsqlb, true ); + expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_stash.recordcount ).toBe( q_native.recordcount ); + }); + + it( title='QoQ select * from table same source table name (all cols) HSQLDB', body=function() { + var q = extensionList(); + var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word + // native engine + systemOutput("native [#cols#]", true); + var q_native = QueryExecute( + sql = "SELECT #cols# FROM q", + options = { dbtype: 'query', maxrows=5 } + ); + var q_stash = duplicate( q_native ); + // hsqldb engine, coz join + systemOutput("hsqldb", true); + var q_hsqlb = QueryExecute( + sql = "SELECT t1.name FROM q_native t1, q_native t2 WHERE t1.id = t2.id", + options = { dbtype: 'query' } + ); + systemOutput( q_hsqlb, true ); + expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); + expect( q_stash.recordcount ).toBe( q_native.recordcount ); + }); + + it( title='QoQ select * from table same source table name (all cols) HSQLDB', body=function() { + var q = extensionList(); + var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word + // native engine + q = QueryExecute( + sql = "SELECT #cols# FROM q", + options = { dbtype: 'query' } + ); + // hsqldb engine, coz join + q = QueryExecute( + sql = "SELECT t1.name FROM q t1, q t2 WHERE t1.id = t2.id", + options = { dbtype: 'query' } + ); + }); + }); + + } + + private function getDummyData (){ + var q = queryNew("id,name,data","integer,varchar, varchar"); + loop list="micha,zac,brad,pothys,gert" item="n" index="i" { + var r = queryAddRow(q); + querySetCell(q, "id", r, r) + querySetCell(q, "name", n, r) + querySetCell(q, "data", repeatString("lucee",1000), r); + } + return q; + } + +} \ No newline at end of file From 874d789e5e33d70eaf6b3deb9f655e361e5cc2c8 Mon Sep 17 00:00:00 2001 From: Zac Spitzer Date: Tue, 16 Jul 2024 11:07:15 +0200 Subject: [PATCH 2/2] LDEV-5023 use a lock around entire QoQ jdbc process --- .../java/lucee/runtime/db/HSQLDBHandler.java | 92 +++++++------------ test/tickets/LDEV5023.cfc | 60 ++++++------ 2 files changed, 66 insertions(+), 86 deletions(-) diff --git a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java b/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java index 2c29a5b927..b75ec6ad30 100644 --- a/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java +++ b/core/src/main/java/lucee/runtime/db/HSQLDBHandler.java @@ -61,7 +61,7 @@ import lucee.runtime.type.StructImpl; import lucee.runtime.type.dt.TimeSpan; import lucee.runtime.type.util.CollectionUtil; -import lucee.aprint; + /** * class to reexecute queries on the resultset object inside the cfml environment */ @@ -301,7 +301,6 @@ private static String toUsableType(int type) { * @throws DatabaseException */ private static void removeTable(Connection conn, String name) throws SQLException { - name = name.replace('.', '_'); Statement stat = conn.createStatement(); stat.execute("DROP TABLE " + name); DBUtil.commitEL(conn); @@ -362,11 +361,8 @@ private static void _executeStatement(Connection conn, String sql) throws SQLExc * @throws DatabaseException */ private static Struct getUsedColumnsForQuery(Connection conn, SQL sql) { - - // TODO this could be potentially cached against the sql text - - Stopwatch stopwatch = new Stopwatch(Stopwatch.UNIT_MILLI); - stopwatch.start(); + // Stopwatch stopwatch = new Stopwatch(Stopwatch.UNIT_MILLI); + // stopwatch.start(); ResultSet rs = null; ResultSetMetaData rsmd = null; String view = "V_QOQ_TEMP"; @@ -374,8 +370,6 @@ private static Struct getUsedColumnsForQuery(Connection conn, SQL sql) { // TODO consider if worth doing, if recordcount / column count is too small - aprint.o(sql.toString()); - try { Statement stat = conn.createStatement(); stat.execute("CREATE VIEW " + view + " AS " + sql.toString()); // + StringUtil.toUpperCase(sql.toString())); @@ -405,8 +399,8 @@ private static Struct getUsedColumnsForQuery(Connection conn, SQL sql) { Struct tableCols = ((Struct) tables.get(tableName)); tableCols.setEL(Caster.toKey(rs.getString(colPos)), null); } - aprint.o(rs); - aprint.o(tables); + // aprint.o(rs); + // aprint.o(tables); // don't need the view anymore, bye bye stat.execute("DROP VIEW " + view); } @@ -469,7 +463,6 @@ public QueryImpl execute(PageContext pc, final SQL sql, int maxrows, int fetchsi } catch (Exception ex) { } - } catch (Exception e) { qoqException = e; @@ -478,7 +471,6 @@ public QueryImpl execute(PageContext pc, final SQL sql, int maxrows, int fetchsi // If our first pass at the QoQ failed, lets look at the exception to see what we want to do with // it. if (qoqException != null) { - // Track the root cause Exception rootCause = qoqException; @@ -559,38 +551,29 @@ public static QueryImpl __execute(PageContext pc, SQL sql, int maxrows, int fetc ConfigPro config = (ConfigPro) pc.getConfig(); DatasourceConnection dc = null; Connection conn = null; - try { - DatasourceConnPool pool = config.getDatasourceConnectionPool(config.getDataSource(QOQ_DATASOURCE_NAME), "sa", ""); - dc = pool.borrowObject(); - conn = dc.getConnection(); - - // executeStatement(conn, "CONNECT"); // create a new HSQLDB session for temp tables - DBUtil.setAutoCommitEL(conn, false); - - // sql.setSQLString(HSQLUtil.sqlToZQL(sql.getSQLString(),false)); + // TODO this is currently single threaded + synchronized (lock) { try { - // we now only lock the data loading, not the execution of the query, but should this be done via - // cflock by the developer? - aprint.out(tables); - synchronized (lock) { + DatasourceConnPool pool = config.getDatasourceConnectionPool(config.getDataSource(QOQ_DATASOURCE_NAME), "sa", ""); + dc = pool.borrowObject(); + conn = dc.getConnection(); + // executeStatement(conn, "CONNECT"); // TODO create a new HSQLDB session for temp tables + DBUtil.setAutoCommitEL(conn, false); + + try { Iterator it = tables.iterator(); - String cfQueryName = null; // name of the source query variable - String dbTableName = null; // name of the table in the database + String cfQueryName = null; // name of the source cfml query variable + String dbTableName = null; // name of the target table in the database String modSql = null; // int len=tables.size(); - SystemOut.print("QoQ HSQLDB CREATED TABLES: " + sql.toString()); while (it.hasNext()) { cfQueryName = it.next().toString();// tables.get(i).toString(); dbTableName = cfQueryName.replace('.', '_'); - if (qoqTables.contains(dbTableName)){ - aprint.o("duplicate table name!!"); - } if (!cfQueryName.toLowerCase().equals(dbTableName.toLowerCase())){ // TODO this could match the wrong strings?? modSql = StringUtil.replace(sql.getSQLString(), cfQueryName, dbTableName, false); sql.setSQLString(modSql); - SystemOut.print("QoQ HSQLDB CREATED TABLES: " + modSql); } if (sql.getItems() != null && sql.getItems().length > 0) sql = new SQLImpl(sql.toString()); // temp tables still get created will all the source columns, @@ -599,7 +582,7 @@ public static QueryImpl __execute(PageContext pc, SQL sql, int maxrows, int fetc qoqTables.add(dbTableName); } - + // SystemOut.print("QoQ HSQLDB CREATED TABLES: " + sql.toString()); // create the sql as a view, to find out which table columns are needed Struct allTableColumns = getUsedColumnsForQuery(conn, sql); @@ -640,32 +623,27 @@ public static QueryImpl __execute(PageContext pc, SQL sql, int maxrows, int fetc } } + catch (SQLException e) { + throw (IllegalQoQException) (new IllegalQoQException("QoQ HSQLDB: error executing sql statement on query.", e.getMessage(), sql, null).initCause(e)); + } } - catch (SQLException e) { - throw (IllegalQoQException) (new IllegalQoQException("QoQ HSQLDB: error executing sql statement on query.", e.getMessage(), sql, null).initCause(e)); - // DatabaseException de = new DatabaseException("QoQ HSQLDB: error executing sql statement on query - // [" + e.getMessage() + "]", null , sql, null); - // throw de; - } - } - catch (Exception ee) { - throw (IllegalQoQException) (new IllegalQoQException("QoQ HSQLDB: error executing sql statement on query.", ee.getMessage(), sql, null).initCause(ee)); - // DatabaseException de = new DatabaseException("QoQ HSQLDB: error executing sql statement on query - // [" + ee.getMessage() + "]", null , sql, null); - // throw ee; - } - finally { - if (conn != null) { - removeAll(conn, qoqTables); - // executeStatement(conn, "DISCONNECT"); // close HSQLDB session with temp tables - DBUtil.setAutoCommitEL(conn, true); + catch (Exception ee) { + throw (IllegalQoQException) (new IllegalQoQException("QoQ HSQLDB: error executing sql statement on query.", ee.getMessage(), sql, null).initCause(ee)); } - if (dc != null) ((DatasourceConnectionPro) dc).release(); + finally { + if (conn != null) { + removeAll(conn, qoqTables); + //executeStatement(conn, "DISCONNECT"); // close HSQLDB session with temp tables + DBUtil.setAutoCommitEL(conn, true); + } + if (dc != null) ((DatasourceConnectionPro) dc).release(); - // manager.releaseConnection(dc); + // manager.releaseConnection(dc); + } + // TODO we are swallowing errors, shouldn't be passing a null value back + if (nqr != null) nqr.setExecutionTime(stopwatch.time()); + return nqr; } - // TOOD we are swalloing errors, shouldn't be passing a null value bacl - if (nqr != null) nqr.setExecutionTime(stopwatch.time()); - return nqr; + } } \ No newline at end of file diff --git a/test/tickets/LDEV5023.cfc b/test/tickets/LDEV5023.cfc index 043eaf74dd..599dabf2a5 100644 --- a/test/tickets/LDEV5023.cfc +++ b/test/tickets/LDEV5023.cfc @@ -5,101 +5,103 @@ component extends="org.lucee.cfml.test.LuceeTestCase" labels="qoq" { describe( title='QofQ' , body=function(){ it( title='QoQ select * from table same source table name HSQLDB', body=function() { - var q = extensionList(); + var q = getTestQuery(); var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word // native engine cols = "name, id"; - systemOutput("native [#cols#]", true); var q_native = QueryExecute( sql = "SELECT #cols# FROM q", options = { dbtype: 'query', maxrows=5 } ); var q_stash = duplicate( q_native ); + expect( q_stash.recordcount ).toBe( q_native.recordcount ); // hsqldb engine, coz join - systemOutput("hsqldb", true); var q_hsqlb = QueryExecute( sql = "SELECT t1.name FROM q_native t1, q_native t2 WHERE t1.id = t2.id", options = { dbtype: 'query' } ); - systemOutput( q_hsqlb, true ); expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_stash.recordcount ).toBe( q_native.recordcount ); }); it( title='QoQ select * from table same source table name (arguments) HSQLDB', body=function() { - var q = extensionList(); + var q = getTestQuery(); var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word // native engine cols = "name, id"; - systemOutput("native [#cols#]", true); var q_native = QueryExecute( sql = "SELECT #cols# FROM q", options = { dbtype: 'query', maxrows=5 } ); var q_stash = duplicate( q_native ); // hsqldb engine, coz join - systemOutput("hsqldb", true); arguments.q_native = q_native; var q_hsqlb = QueryExecute( sql = "SELECT t1.name FROM q_native t1, arguments.q_native t2 WHERE t1.id = t2.id", options = { dbtype: 'query' } ); - systemOutput( q_hsqlb, true ); + expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_stash.recordcount ).toBe( q_native.recordcount ); }); it( title='QoQ select * from table same source table name (all cols) HSQLDB', body=function() { - var q = extensionList(); + var q = getTestQuery(); var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word // native engine - systemOutput("native [#cols#]", true); var q_native = QueryExecute( sql = "SELECT #cols# FROM q", options = { dbtype: 'query', maxrows=5 } ); var q_stash = duplicate( q_native ); // hsqldb engine, coz join - systemOutput("hsqldb", true); var q_hsqlb = QueryExecute( sql = "SELECT t1.name FROM q_native t1, q_native t2 WHERE t1.id = t2.id", options = { dbtype: 'query' } ); - systemOutput( q_hsqlb, true ); expect( q_stash.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_native.recordcount ).toBe( q_hsqlb.recordcount ); expect( q_stash.recordcount ).toBe( q_native.recordcount ); }); - it( title='QoQ select * from table same source table name (all cols) HSQLDB', body=function() { - var q = extensionList(); - var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word - // native engine - q = QueryExecute( - sql = "SELECT #cols# FROM q", - options = { dbtype: 'query' } - ); - // hsqldb engine, coz join - q = QueryExecute( - sql = "SELECT t1.name FROM q t1, q t2 WHERE t1.id = t2.id", - options = { dbtype: 'query' } - ); + it( title='QoQ select * from table same source table name (all cols) HSQLDB, 5000 threads', body=function() { + + var arr = []; + ArraySet(arr, 1, 1000, 0); + arrayEach(arr, function(){ + var q = getTestQuery(); + var cols = replaceNoCase( q.columnList, ",unique", "" ); // cleanup reserved word + // native engine + q = QueryExecute( + sql = "SELECT #cols# FROM q", + options = { dbtype: 'query' } + ); + // hsqldb engine, coz join + q = QueryExecute( + sql = "SELECT t1.name FROM q t1, q t2 WHERE t1.id = t2.id", + options = { dbtype: 'query' } + ); + }, true); + }); }); } - private function getDummyData (){ + private function getTestQuery(){ + return extensionList(); // has arrays and the like + /* var q = queryNew("id,name,data","integer,varchar, varchar"); - loop list="micha,zac,brad,pothys,gert" item="n" index="i" { - var r = queryAddRow(q); + loop list="micha,zac,brad,pothys,gert" item="local.n" index="local.i" { + var r = queryAddRow( q ); querySetCell(q, "id", r, r) querySetCell(q, "name", n, r) - querySetCell(q, "data", repeatString("lucee",1000), r); + //querySetCell(q, "data", repeatString("lucee",1000), r); } return q; + */ } } \ No newline at end of file