PostgreSQL column queries parse generated SQL instead of using the attribute object
Summary
changeColumnQuery and addColumnQuery on PostgreSQL reconstruct a column definition by parsing the SQL string that attributeToSQL produced, rather than working from the attribute object. Column comments make this worse, because attributeToSQL smuggles a whole second statement (; COMMENT ON COLUMN ...) into that same string.
This issue is to record the problem and collect options. No direction has been decided yet. Please treat the sketch below as one candidate, not a plan of record.
Why this keeps coming up
A string carrying both a type definition and a trailing statement is then scanned for the substrings NOT NULL, DEFAULT, REFERENCES, UNIQUE, PRIMARY KEY, SERIAL and ^ENUM\(.+\). Anything a user puts in a comment, an enum value or a default literal can collide with one of those.
Issues in this family:
- #17894:
USINGclause ends up attached toCOMMENT ON COLUMNwhen altering an enum - #17118:
addColumnwith an enum and a comment ending in)producesunterminated quoted string - #17544: an enum value containing
PREFERENCESis parsed asREFERENCESand turned into a foreign key (MySQL)
Current state
Both comment-direction bugs are addressed in #18254, which now covers changeColumn (#17894) and addColumn (#17118). It splits the comment statement off the definition before any type parsing runs, so comment text never reaches the regexes. That closed the whole class rather than one case: on main a comment containing PRIMARY KEY or NOT NULL was silently rewritten and, in the NOT NULL case, silently made the column non-nullable. Six of seven new regression cases fail on main and pass on that branch.
Separately, #18361 fixes two changeColumnQuery bugs that had nothing to do with comments: ARRAY(ENUM) emitted a scalar USING cast, and SET DEFAULT was emitted before the type change.
So the acute failures are handled. What remains is structural: the string round-trip is still there, and #17544 is still open. Nothing below is urgent.
One candidate direction
Pass the normalized attribute object down instead of only the SQL string.
QueryInterface#changeColumn already computes this.normalizeAttribute(dataTypeOrOptions) and then discards it (packages/core/src/abstract-dialect/query-interface.js). Handing that object to changeColumnQuery as an optional extra argument would let the PostgreSQL generator:
- emit
COMMENT ON COLUMNfromattribute.commentdirectly, instead of parsing it back out of a string - call
pgEnumwith theDataTypes.ENUMinstance, a branchpgEnumalready supports - know whether the attribute is an
ARRAY(ENUM)without astartsWith('ENUM(')string test
addColumnQuery already receives the attribute object and is the closest existing model. MSSQL does something similar with its commentTemplate helper.
Concretely, the call site changes from passing only the SQL map to passing the attributes alongside it:
// packages/core/src/abstract-dialect/query-interface.js
const normalizedAttributes = { [attributeName]: this.normalizeAttribute(dataTypeOrOptions) };
const query = this.queryGenerator.attributesToSQL(normalizedAttributes, {
context: 'changeColumn',
table: tableName,
});
const sql = this.queryGenerator.changeColumnQuery(tableName, query, normalizedAttributes);and the PostgreSQL generator reads the attribute instead of the string:
// packages/postgres/src/query-generator.js
changeColumnQuery(tableName, attributes, normalizedAttributes) {
for (const attributeName in attributes) {
const attribute = normalizedAttributes?.[attributeName];
const dataType = attribute?.type;
// comment comes from the object, not from parsing the definition
const columnComment = attribute?.comment
? this.#columnCommentQuery(tableName, attributeName, attribute.comment)
: '';
// enum type comes from the DataType instance, not from `startsWith('ENUM(')`
if (dataType instanceof DataTypes.ENUM) { /* ... */ }
else if (dataType instanceof DataTypes.ARRAY
&& dataType.options.type instanceof DataTypes.ENUM) { /* ... */ }
}
}When normalizedAttributes is absent the generator falls back to today's string parsing, so direct callers of changeColumnQuery keep working and the public queryInterface.changeColumn signature does not change.
Blast radius: the PostgreSQL generator, one line in the core query interface, and the PostgreSQL fixtures.
Other options worth weighing
- Do nothing further. The known failures are fixed and the remaining risk is a contrived enum value colliding with the sentinel.
- Do the same object-based change across all dialects at once, alongside the ongoing migration of query generators to the TypeScript base class. Larger blast radius, but avoids doing this twice.
- Narrow the problem instead: stop having
attributeToSQLemit comment statements at all, and make comments the caller's responsibility in every path that needs them.
Adjacent gaps found while testing
Not part of this issue, recorded so they are not lost.
ColumnDescription does not declare special. PostgreSQL's describeTable really does return special with the enum values, and the .js tests assert it freely, but the interface in packages/core/src/abstract-dialect/query-interface.types.ts only declares type, allowNull, defaultValue, primaryKey, autoIncrement and comment. Any .ts test that asserts special fails the TS Typings job with TS2339, which is exactly what happened on #18254. The .js suites never catch it because they are not type-checked.
If we add it, #18303 is already extending the same interface with optional generatedAs and generatedColumn, so that PR is both the precedent for the shape and the thing to sequence against. Note special is dialect-specific, so special?: string[] is probably right rather than a required field.
Other gaps, with their existing owners where there is one:
- There is no
dialect.supportsflag for column comments, so tests that exercise them have to name dialects.describeTable.test.jshardcodes['postgres', 'mysql', 'mssql']. describeTablereturns an emptyspecialforARRAY(ENUM)columns, so enum values are not reported for array columns. Related to #15466, where PostgreSQL returns enum arrays as raw strings because no type parser is registered for the generated array type.addColumnQueryon db2 emits an unquotedCOMMENT Status (active/pending), which is invalid SQL, and sqlite3 and ibmi drop column comments silently. #18289 tracks the db2 side and #18309 is the in-flight fix.
Already tracked separately, listed so this issue does not duplicate them: #16787 (inconsistent defaultValue parsing across dialects) and #17288 (PostgreSQL describeTable breaking with column comments and same-named tables in different schemas).
Open questions
- Is this worth doing at all now that the acute bugs are fixed?
- If yes, per-dialect (PostgreSQL first) or as part of the wider generator migration?
- Is an optional extra argument on
changeColumnQueryacceptable, or should the signature change outright given it is an internal API?
Filed by Claude Code (Claude Opus 5).
Source: sequelize/sequelize