Repository navigation
docs: note where generated migrations do not escape resource SQL - #880
Merged
Merged
Conversation
The migration generator writes some SQL from the resource into the
migration's Elixir source as is, so Elixir reinterprets backslash escapes
and `#{` in it, and a `"` can end a double-quoted string. Fixing that is a
breaking change for anyone who escapes the SQL themselves, so this marks
each site with a `3.0:` comment and documents the current behaviour on the
options that reach them: `check`, custom statement `up` and `down`,
`base_filter_sql`, `identity_wheres_to_sql` and `calculations_to_sql`.
Refs ash-project#876.
Contributor
|
Perfect 👍 🚀 Thank you for your contribution! 🚀 |
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
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.
Follow-up to #876, as you suggested there: a
3.0:comment at each place the migration generator writes resource SQL into the migration without escaping it, and docs for the options that reach those places. No behaviour changes.To find the places, I went through everything in
lib/migration_generator/(and the helpers it calls) that writes text from the resource into the migration source, then checked each SQL site against the generator with a backslash, a"and#{in the input.3.0:comments (lib/migration_generator/operation.ex)AddCheckConstraint.up/1check,base_filter_sqlcheck: """heredocRemoveCheckConstraint.down/1AddCustomStatement.up/1,down/1(code?: false)up,downexecute("""heredocAddUniqueIndex.up/1, base filter without an identitywherebase_filter_sqlwhere: "..."AddUniqueIndex.up/1, temporal exclusion constraintidentity_wheres_to_sql,base_filter_sql,calculations_to_sqlexecute("...")Docs
check(check constraints),upanddown(custom statements), andbase_filter_sql,identity_wheres_to_sqlandcalculations_to_sql(thepostgressection) now say how the SQL is written into the migration and how to escape it, withcheck: ~S(code ~ '^\\d{4}$')as the example.documentation/dsls/DSL-AshPostgres.DataLayer.mdis regenerated.Things that came up, for you to decide
execute("...")as is. A quoted identifier in an identity'swhereSQL makes the generated migration fail to parse, and\sincalculations_to_sqlreaches Postgres as a space. The non-temporal branches escape the same SQL throughoption/2. Since this shipped in 2.14.0, it may be early enough to escape it now rather than in 3.0; I can send that separately if you'd like.base_filter_sqlis escaped in some places and not others. It is escaped in custom indexes and in an identity's unique index when the identity has awhere, but not in check constraints, an identity's unique index without awhere, or temporal identities. So on a resource with both kinds of site, no single way of writing a base filter that contains a backslash comes out right everywhere. The docs say so.~S"""heredoc still ends at SQL that has"""at the start of a line.SerialSequenceTransitionrenders throughAddCustomStatementand relies on#{prefix()}being interpolated at runtime, so escaping there has to leave those statements alone. The comment there says so.match_withcolumns and extension names are also written raw into someexecute("...")andname: "..."strings, for example inAddPrimaryKey,RenameUniqueIndex,AlterDeferrability,AddTemporalForeignKeyand the extension migrations. They only matter for names with unusual characters, so I left them out; happy to mark them too.Checks
Run on
mainat c0686d2:mix format --check-formatted,mix spark.cheat_sheets --check,mix spark.formatter --check,mix credo --strictandmix sobelowall pass. Dialyzer was not run locally; this change only adds comments and docs.test/migration_generator_test.exsandtest/temporal_migration_generator_test.exspass (120 tests).UniqAggregateSortTestx2,JoinSubquerySortTest,AshSql.AggregateTest) fail the same way onmainwithout this change.Contributor checklist
Leave anything that you believe does not apply unchecked.