Conversation
core/src/main/kotlin/org/evomaster/core/solver/SMTLibZ3DbConstraintSolver.kt
Outdated
Show resolved
Hide resolved
|
hi @agusaldasoro , thx. first ask @jgaleotti for a review. once he has given it, and you address it, then I can be asked for my review :) |
| * Characters that decompose under NFD (Å→A, and other accented letters like é, ü, ñ) | ||
| * are handled by normalizing to NFD form and stripping the remaining non-ASCII combining marks. | ||
| */ | ||
| fun sanitizeSmtIdentifier(name: String): String { |
There was a problem hiding this comment.
I think this belongs to org.evomaster.core.utils.StringUtils, perhapss a more suitable method name could be transliterateIdentifier()
| val genes = mutableListOf<Gene>() | ||
| for (columnName in columns.fields) { | ||
| var gene: Gene = IntegerGene(columnName, 0) | ||
| // columnName is the sanitized (ASCII) version from Z3; resolve back to original DB column name |
There was a problem hiding this comment.
I am not sure the right term for this should be sanitized
| // Only add GetValueSMTNode for the mentioned tables | ||
| for (table in schema.tables) { | ||
| val tableNameLower = table.id.name.lowercase() | ||
| val sanitizedTableNameLower = sanitizeSmtIdentifier(tableNameLower) |
There was a problem hiding this comment.
I am not sure the right term should be "sanitized"
| val originalColumn = table.columns.firstOrNull { | ||
| SmtLibGenerator.sanitizeSmtIdentifier(it.name).equals(columnName, ignoreCase = true) | ||
| } | ||
| val actualColumnName = originalColumn?.name ?: columnName |
There was a problem hiding this comment.
actualColumnName -> columnName?
| @@ -153,36 +153,41 @@ class SMTLibZ3DbConstraintSolver() : DbConstraintSolver { | |||
| // Create the list of genes with the values | |||
| val genes = mutableListOf<Gene>() | |||
| for (columnName in columns.fields) { | |||
There was a problem hiding this comment.
columnName -> smtColumnName?
| private fun appendTableDefinitions(smt: SMTLib) { | ||
| for (table in schema.tables) { | ||
| val dataTypeName = "${StringUtils.capitalization(table.id.name)}Row" | ||
| val sanitizedTableName = convertToAscii(table.id.name) |
There was a problem hiding this comment.
sanitizedTableName -> smtTableName?
| */ | ||
| private fun appendUniqueConstraints(smt: SMTLib, table: TableDto) { | ||
| val tableName = table.id.name.lowercase() | ||
| val tableName = convertToAscii(table.id.name).lowercase() |
There was a problem hiding this comment.
tableName -> smtTableName?
| */ | ||
| private fun appendPrimaryKeyConstraints(smt: SMTLib, table: TableDto) { | ||
| val tableName = table.id.name.lowercase() | ||
| val tableName = convertToAscii(table.id.name).lowercase() |
There was a problem hiding this comment.
tableName -> smtTableName ?
| */ | ||
| private fun getColumnReference(tableName: String, columnName: String): String { | ||
| return "(${columnName.uppercase()} ${tableName.lowercase()}$rowIndex)" | ||
| return "(${convertToAscii(columnName).uppercase()} ${convertToAscii(tableName).lowercase()}$rowIndex)" |
There was a problem hiding this comment.
add a comment to explain why this needs to be converted to ASCII
| @@ -131,7 +132,7 @@ class SmtLibGenerator(private val schema: DbInfoDto, private val numberOfRows: I | |||
| * @return The corresponding SMT node. | |||
| */ | |||
| private fun parseCheckExpression(table: TableDto, condition: SqlCondition, index: Int): SMTNode { | |||
There was a problem hiding this comment.
maybe not for this PR, but something to discuss as well with @jgaleotti , why is most of the code here relaying on TableDto and not org.evomaster.core.sql.schema.Table. DTOs are only meant as simple POJOs for transfer of data between processes, and not for business logic computation
There was a problem hiding this comment.
I did a draft PR with that change, I'm not sure if the interface for fun solve(schemaDto: DbInfoDto, sqlQuery: String, numberOfRows: Int): List<SqlAction> should remain the same or we should change it too.
| private fun appendTimestampConstraints(smt: SMTLib) { | ||
| for (table in schema.tables) { | ||
| val tableName = table.id.name.lowercase() | ||
| val smtTableName = convertToAscii(table.id.name).lowercase() |
There was a problem hiding this comment.
there are so many calls to this convertToAscii, which makes it more difficult to read, and possibly easy to make mistake in future if code needs changing and missing a needed call. maybe create a new SmtTable class with that that you need, and pass that around the code instead of doing the conversions each time?
| fun convertToAscii(name: String): String { | ||
| val replaced = name | ||
| .replace('Ø', 'O').replace('ø', 'o') | ||
| .replace("Æ", "AE").replace("æ", "ae") |
There was a problem hiding this comment.
what about Norwegian å and Å? or Swedish characters like Ä, ä, Ö and ö ? or other special characters in all other languages?
we cannot have ad-hoc solutions like this, only specific to our test data. Need a general solution.
This pull request refactors and improves the handling of table and column names in the SMT-LIB generation logic, ensuring consistent ASCII conversion and proper mapping between original database schema names and their SMT-LIB representations. The changes introduce a new
SmtTableabstraction to encapsulate SMT-specific naming, update all relevant logic to use this abstraction, and fix issues related to non-ASCII schema names. The most important changes are as follows:SMT-LIB Name Handling and Abstraction:
SmtTableabstraction inSmtLibGeneratorto encapsulate SMT-LIB-specific names and maintain a mapping between original schema names and their ASCII-converted SMT-LIB counterparts, ensuring consistent handling of table and column names throughout the SMT generation process. [1] [2]Consistent ASCII Conversion:
Constraint Generation Refactor:
SmtTableabstraction and ASCII-converted names, replacing direct access to the original schema DTOs. [1] [2] [3] [4] [5] [6] [7] [8]Query Condition Parsing:
Gene Mapping Fixes:
SMTLibZ3DbConstraintSolverto resolve SMT-LIB column names back to their original database column names, ensuring correct gene creation and type handling for database operations.These changes collectively improve the robustness and correctness of SMT-LIB constraint generation, especially for schemas with non-ASCII identifiers.