refactor: remove dead predicate .sql() path; hoist import; escape Literal repr - #4
Merged
Merged
Conversation
…eral repr D1: Predicate.sql(), CompoundPredicate.sql(), and NotPredicate.sql() were dead and dialect-blind — only ever called by each other, hardcoding "?" placeholders and unqualified column names, contradicting the Dialect invariant the real compile path (Compiler._compile_predicate) upholds. Remove all three; the public BaseRelation.sql() is unaffected. D2: Hoist SQL_OPERATORS into the module-level predicates import and drop the function-local import in Compiler._compile_predicate. D3: Literal.__repr__ now escapes embedded double-quotes so a string value containing one renders balanced in algebra notation; the stale comment that referenced the removed Predicate.sql() is updated. Adds TestPredicateDeadCodeRemoval. Closes D1, D2, D3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P3 dead code & cleanup (closes D1, D2, D3)
D1 — remove dead, dialect-blind
Predicate.sql()familyPredicate.sql(),CompoundPredicate.sql(), andNotPredicate.sql()wereonly ever called by each other — nothing outside the predicate hierarchy
invoked them. The real compile path is
Compiler._compile_predicate. The deadmethods hardcoded
?placeholders and unqualified column names, contradictingthe "all SQL flows through the Dialect" invariant and trapping any maintainer
who reached for
pred.sql(). All three removed; the publicBaseRelation.sql()(which goes through the compiler) is unaffected.D2 — hoist function-local import
Compiler._compile_predicatedidfrom .predicates import SQL_OPERATORSinside the function, though
predicatesis already imported at module top.Hoisted into the top-level import.
D3 — escape
Literal.__repr__A string literal containing
"rendered unbalanced in algebra notation(
f'"{value}"'). Now escapes the embedded quote. Plain strings areunchanged (
city="London"), and the stale comment that referenced theremoved
Predicate.sql()is corrected.Tests
TestPredicateDeadCodeRemoval: embedded-quote literal renders balanced;plain strings/numbers unchanged; predicate classes expose no
.sql();end-to-end
AND/NOTselections still compile through the real path.Local gate green:
ruff,mypy,pytest(110 passed, 15 skipped).