sqlalchemy / sqlalchemy/alembic
Consider Removing New Line Characters when Comparing Computed Constructs
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 4.4k
- Forks
- 375
- PR merge metrics
- No merged PRs in 30d
Description
I'm working with a Computed Column construct, which is really long and therefore contains line breaks for readability.
When this construct gets imported into the underlying database (Postgres in my case), the construct gets reformatted with line breaks inserted in different places.
This creates a small side-effect, where alembic now thinks that the original computed clause provided by the migration file is different to the one in the database, and therefore emits a warning.
/usr/local/lib/python3.10/site-packages/alembic/autogenerate/compare.py:1090:
UserWarning: Computed default on *** cannot be modified
util.warn("Computed default on %s.%s cannot be modified" % (tname, cname))
After inspecting the code, I found this method, which appears to sanitize the database computed clause as well as the local one:
I feel that we should include line breaks in the list of tokens that should be removed before a comparison takes place, but happy to hear what thoughts you have on this.
Thanks!
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in alembic/autogenerate/compare.py around lines 1047-1055, where the database and local computed clauses are sanitized before comparison. Reproduce the issue with a multiline Computed Column against PostgreSQL, then verify that equivalent clauses with differently placed line breaks no longer produce the computed-default warning.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, python
- Domain
- database
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 74/100