fix: resolve the object type behind modifiers in statement_type - #301
Conversation
|
This pull request is part of a Mergify stack:
|
Merge Protections🟢 All 6 merge protections satisfied — ready to merge. Show 6 satisfied protections🟢 🤖 Continuous Integration
🟢 👀 Review Requirements
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 Reviews
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
|
@sileht this pull request is now in conflict 😩 |
db8670e to
5a9c785
Compare
Revision history
|
5a9c785 to
bc14ac6
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The dependency bump is well-scoped and the tests are updated to explicitly pin the relevant externally visible behavior changes.
Pull request overview
This PR upgrades the project’s sqlparse dependency to 0.6.0 to pick up security/DoS fixes, and updates tests to pin the observable Statement.statement_type behavior changes introduced by the new lexer keywording.
Changes:
- Bump
sqlparseminimum version to>=0.6.0and update the lockfile to0.6.0. - Extend
test_statement_typecoverage to pinMATERIALIZED/TEMPORARYstatement-type outcomes under the updated lexer behavior.
File summaries
| File | Description |
|---|---|
| tests/test_sql_compare.py | Adds/updates statement type expectations for MATERIALIZED VIEW and TEMPORARY TABLE cases under sqlparse 0.6.0. |
| pyproject.toml | Raises the minimum supported sqlparse version to >=0.6.0. |
| poetry.lock | Locks sqlparse to 0.6.0 and updates associated metadata/hashes. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bc14ac6 to
18e652b
Compare
18e652b to
0f64cce
Compare
|
@sileht this pull request is now in conflict 😩 |
0f64cce to
83fb20f
Compare
`Statement.statement_type` took the first two keywords, so any keyword sitting between the verb and the object displaced the object out of the window. sqlparse 0.6.0 made that visible by lexing MATERIALIZED as a keyword, but it predates the bump: TEMPORARY, TEMP and UNIQUE were already doing it. CREATE TEMPORARY TABLE "CREATE TEMPORARY" -> "CREATE TABLE" CREATE TEMP TABLE "CREATE TEMP" -> "CREATE TABLE" CREATE UNIQUE INDEX "CREATE UNIQUE" -> "CREATE INDEX" CREATE MATERIALIZED VIEW "CREATE MATERIALIZED"-> "CREATE MATERIALIZED VIEW" DROP MATERIALIZED VIEW "DROP MATERIALIZED" -> "DROP MATERIALIZED VIEW" ALTER MATERIALIZED VIEW "ALTER MATERIALIZED" -> "ALTER MATERIALIZED VIEW" REFRESH MATERIALIZED VIEW "MATERIALIZED" -> "REFRESH MATERIALIZED VIEW" CREATE OR REPLACE VIEW "CREATE OR REPLACE" -> "CREATE OR REPLACE VIEW" Three kinds of word are now told apart: modifiers that do not change what the object is and are dropped (TEMPORARY, TEMP, GLOBAL, LOCAL, UNIQUE, RECURSIVE); qualifiers that are part of the object's name and let it continue (MATERIALIZED); and verbs sqlparse does not lex as keywords at all, recovered from the statement's first token (REFRESH, which came through with ttype None and left the type headless). `OBJECT_VERBS` deliberately holds only verbs that are always followed by the keyword naming their object. TRUNCATE acts on a name, so pairing it with the next keyword would read a trailing option as the object type and invent "TRUNCATE CASCADE", "TRUNCATE RESTART" and friends; it stays out, and tests pin that. This fixes one comparison, not just the labels. "CREATE TEMPORARY" was not in `UnorderedTokenList.STATEMENT_TYPES`, so temporary tables never had their column lists sorted and two identical ones written in a different column order compared as drifted. They now behave like plain CREATE TABLE. Temporary tables that genuinely differ still compare unequal, statements with no keywords still resolve to UNKNOWN so `_parse_statements` filters exactly what it filtered before, and a statement behind a leading comment is still typed from its keywords. Claude-Session: https://claude.ai/code/session_01XkWQRxh5pfi2Fbuv3Z9wen Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Change-Id: If5b05d1f96310d93905b8f759e12d84f6f1d363f
83fb20f to
35642ab
Compare
Merge Queue Status
This pull request spent 47 seconds in the queue, including 9 seconds running CI. Required conditions to merge
|
Statement.statement_typetook the first two keywords, so any keywordsitting between the verb and the object displaced the object out of the
window. sqlparse 0.6.0 made that visible by lexing MATERIALIZED as a
keyword, but it predates the bump: TEMPORARY, TEMP and UNIQUE were
already doing it.
CREATE TEMPORARY TABLE "CREATE TEMPORARY" -> "CREATE TABLE"
CREATE TEMP TABLE "CREATE TEMP" -> "CREATE TABLE"
CREATE UNIQUE INDEX "CREATE UNIQUE" -> "CREATE INDEX"
CREATE MATERIALIZED VIEW "CREATE MATERIALIZED"-> "CREATE MATERIALIZED VIEW"
DROP MATERIALIZED VIEW "DROP MATERIALIZED" -> "DROP MATERIALIZED VIEW"
ALTER MATERIALIZED VIEW "ALTER MATERIALIZED" -> "ALTER MATERIALIZED VIEW"
REFRESH MATERIALIZED VIEW "MATERIALIZED" -> "REFRESH MATERIALIZED VIEW"
CREATE OR REPLACE VIEW "CREATE OR REPLACE" -> "CREATE OR REPLACE VIEW"
Three kinds of word are now told apart: modifiers that do not change what
the object is and are dropped (TEMPORARY, TEMP, GLOBAL, LOCAL, UNIQUE,
RECURSIVE); qualifiers that are part of the object's name and let it
continue (MATERIALIZED); and verbs sqlparse does not lex as keywords at
all, recovered from the statement's first token (REFRESH, which came
through with ttype None and left the type headless).
OBJECT_VERBSdeliberately holds only verbs that are always followed bythe keyword naming their object. TRUNCATE acts on a name, so pairing it
with the next keyword would read a trailing option as the object type and
invent "TRUNCATE CASCADE", "TRUNCATE RESTART" and friends; it stays out,
and tests pin that.
This fixes one comparison, not just the labels. "CREATE TEMPORARY" was
not in
UnorderedTokenList.STATEMENT_TYPES, so temporary tables neverhad their column lists sorted and two identical ones written in a
different column order compared as drifted. They now behave like plain
CREATE TABLE. Temporary tables that genuinely differ still compare
unequal, statements with no keywords still resolve to UNKNOWN so
_parse_statementsfilters exactly what it filtered before, and astatement behind a leading comment is still typed from its keywords.
Claude-Session: https://claude.ai/code/session_01XkWQRxh5pfi2Fbuv3Z9wen
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com