Addind SQL mutations - #1656
Conversation
| MAP = REPLACEMENTS.to_h.freeze | ||
|
|
||
| # Combined pattern matching SQL keywords to mutate. | ||
| PATTERN = ::Regexp.new( |
There was a problem hiding this comment.
@bsene Thank you for your contribution, but I think regexp based syntax modification will never work at scale. It has to many edge cases. What if an SQL literal contains the string OR and all of the other interesting edge cases.
For mutant to gain proper SQL mutations a proper AST should be used. pg_query (gem) comes to mind it would have to be paired with an SQL unparser. Just like I made the ruby unparser gem to power mutant.
Would you consider to change this PR to use a proper SQL AST parser/unparser?
There was a problem hiding this comment.
Yes fair point, I rewrote this with a real SQL/AST tool (eg: pg_query as you suggested).
What do you think ?
There was a problem hiding this comment.
The segfault in pg_query's unparser on malformed operands is genuinely an upstream bug: a deparser should never crash, it should raise or return empty.
-> I'll submit an issue against pg_query.
The ignore: entry is just a pragmatic workaround: those degenerate trees are only produced by mutant's self-mutation of wrap_exists, never through the public mutate(sql) API, so no real test can distinguish them IMO.
There was a problem hiding this comment.
@bsene if there is one crash there are many. You have an idea about a source of diverse SQL Statements we can hook up to the corpus tests to proof we filtered out the "only" segfault?
There was a problem hiding this comment.
I rely on those SQL statements.
My strategy was first focus on small set of operator flips , fix them then complete the corpus as you suggested although that should belongs to the pg_query internal tests (if there are present) -> I'll check it soon
…nuation The chained .select/.map/.join continuation was indented 4 spaces, making it inconsistent with the surrounding 2-space Ruby style. Re-align to the lower continuation indent so the block reads as one pipeline.
Replace the regex-based SQL keyword flipping with a real SQL AST: parse the heredoc body with pg_query, flip one typed node at a time (AND/OR, IS NULL/IS NOT NULL, IN/NOT IN, ASC/DESC, EXISTS/NOT EXISTS), and deparse back to SQL text. This avoids the edge cases a regex cannot handle (e.g. "OR" inside a string literal), per review feedback. Parse failures yield no SQL mutations (graceful, no crash).
| - Mutant::Mutator::Node::BlockPass#dispatch | ||
| # NOT-wrap mutator: mutations that strip the wrapped operand (args:[node]→ | ||
| # nil/[], or dropping bool_expr/boolop) yield a NOT with no operand, which | ||
| # pg_query's native deparser cannot rebuild (it segfaults) or drops to a bare |
There was a problem hiding this comment.
mhh this makes me very vary about the quality of pg_query ans this segfault would take mutant with it.
Sure while you work around this segfault, there is a high likelyness ther are other segfaults still hiding.
I wounder if we should add a very SQL heavy corpus test before merging to avoid shipping a mutant version that segfaults on yet to be learned SQL?
2 participants
resolve issue #1360