From 06ff3cf764333b28e7c0c430113c58472e61d3df Mon Sep 17 00:00:00 2001 From: Oleg Babichev Date: Mon, 3 Jun 2024 13:23:27 +0200 Subject: [PATCH] Review issues: add logger for the failing test, add logging of sql and list of columns on the cached exception --- .../exposed/sql/statements/InsertStatement.kt | 40 +++++++++++-------- .../org/jetbrains/exposed/dao/EntityCache.kt | 6 +-- .../sql/tests/shared/entities/EntityTests.kt | 1 + 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt index 1558ffdad3..b273a31078 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt @@ -2,6 +2,7 @@ package org.jetbrains.exposed.sql.statements import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi +import org.jetbrains.exposed.sql.transactions.TransactionManager import org.jetbrains.exposed.sql.vendors.* import org.jetbrains.exposed.sql.vendors.inProperCase import java.sql.ResultSet @@ -68,25 +69,32 @@ open class InsertStatement( val firstAutoIncColumn = autoIncColumns.firstOrNull { it.autoIncColumnType != null } ?: autoIncColumns.firstOrNull() if (firstAutoIncColumn != null || returnedColumns.isNotEmpty()) { while (rs?.next() == true) { - val returnedValues = returnedColumns.associateTo(mutableMapOf()) { it.first to rs.getObject(it.second) } - if (returnedValues.isEmpty() && firstAutoIncColumn != null) { - try { + try { + val returnedValues = returnedColumns.associateTo(mutableMapOf()) { it.first to rs.getObject(it.second) } + if (returnedValues.isEmpty() && firstAutoIncColumn != null) { returnedValues[firstAutoIncColumn] = rs.getObject(1) - } catch (e: ArrayIndexOutOfBoundsException) { - // EXPOSED-191 Flaky Oracle test on TC build - // this try/catch should help to get information about the flaky test. - // try/catch can be safely removed after the fixing the issue. - // TooGenericExceptionCaught suppress also can be removed - - exposedLogger.error( - "ArrayIndexOutOfBoundsException on processResults. " + - "Table: ${table.tableName}, firstAutoIncColumn: ${firstAutoIncColumn.name}, inserted: $inserted", - e - ) - throw e } + autoGeneratedKeys.add(returnedValues) + } catch (cause: ArrayIndexOutOfBoundsException) { + // EXPOSED-191 Flaky Oracle test on TC build + // this try/catch should help to get information about the flaky test. + // try/catch can be safely removed after the fixing the issue. + // TooGenericExceptionCaught suppress also can be removed + + val preparedSql = this.prepareSQL(TransactionManager.current(), prepared = true) + + val returnedColumnsString = returnedColumns + .mapIndexed { index, pair -> "column: ${pair.first.name}, index: ${pair.second} (columns-list-index: $index)" } + .joinToString(prefix = "[", postfix = "]", separator = ", ") + + exposedLogger.error( + "ArrayIndexOutOfBoundsException on processResults. " + + "Table: ${table.tableName}, firstAutoIncColumn: ${firstAutoIncColumn?.name}, inserted: $inserted, returnedColumnsString: $returnedColumnsString. " + + "Failed SQL: $preparedSql", + cause + ) + throw cause } - autoGeneratedKeys.add(returnedValues) } if (inserted > 1 && firstAutoIncColumn != null && autoGeneratedKeys.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { diff --git a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt index 0387e7b2eb..01bc02ac0d 100644 --- a/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt +++ b/exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt @@ -245,7 +245,7 @@ class EntityCache(private val transaction: Transaction) { } } } - } catch (e: ArrayIndexOutOfBoundsException) { + } catch (cause: ArrayIndexOutOfBoundsException) { // EXPOSED-191 Flaky Oracle test on TC build // this try/catch should help to get information about the flaky test. // try/catch can be safely removed after the fixing the issue @@ -255,8 +255,8 @@ class EntityCache(private val transaction: Transaction) { entry.writeValues.map { writeValue -> "${writeValue.key.name}=${writeValue.value}" }.joinToString { ", " } } - exposedLogger.error("ArrayIndexOutOfBoundsException on attempt to make flush inserts. Table: ${table.tableName}, entries: ($toFlushString)", e) - throw e + exposedLogger.error("ArrayIndexOutOfBoundsException on attempt to make flush inserts. Table: ${table.tableName}, entries: ($toFlushString)", cause) + throw cause } for ((entry, genValues) in toFlush.zip(ids)) { diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/EntityTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/EntityTests.kt index b117042918..9dd1ac99d0 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/EntityTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/EntityTests.kt @@ -557,6 +557,7 @@ class EntityTests : DatabaseTestsBase() { @Test fun callLimitOnRelationDoesntMutateTheCachedValue() { withTables(Posts, Boards, Categories) { + addLogger(StdOutSqlLogger) val category1 = Category.new { title = "cat1" }