Fixed Issue #3323

[Hydrocharged]

Generated column expressions now survive ALTER TABLE ADD PRIMARY KEY.

• Generated column with COALESCE: every INSERT fails after ALTER TABLE ADD PRIMARY KEY #3323

[itoqa]

Commit: 5a73ebf: 11 test cases ran, 9 passed ✅, 2 additional findings ⚠️.

Summary

Coverage spans generated and default column behavior through schema changes, expression evaluation, metadata handling, retry and recovery paths after rejected database changes, and identity-column edge cases. The exercised flows include normal inserts and updates plus malformed or unsupported definitions, with the main generated-column behaviors healthy and two broader catalog/identity limitations observed.

Safe to merge — the failures are pre-existing identity and metadata limitations, not regressions or new failures caused by this PR, and the tested behavior relevant to the change remains healthy. These issues are worth tracking separately but are not merge blockers for this change.

Tests run by Ito

Result	Severity	Type	Description
✅	—	General	Adding a generated column and a default column keeps the column order, metadata, and values correct for old and new rows. Direct writes to the generated column are rejected as expected.
✅	—	General	The invalid later argument was rejected, and retrying with valid arguments created the expression and returned 29.
✅	—	General	The malformed column change was rejected, and the table stayed usable. A valid retry added only the intended generated column, which returned 11 and 12 for the two rows.
✅	—	General	The invalid generated-column request returned a clear error and did not leave a partial column behind. A valid request then succeeded on the same connection, and new rows received the correct generated values.
✅	—	Expression	Creating a stored generated column with upper(a::text) succeeded, and inserting 12 returned the generated value 12.
✅	—	Expression	A generated column using two input columns was created successfully, and inserting 2 and 9 produced 29.
✅	—	Generated	Both tables accepted a new column and later inserts kept the generated and default values correct.
✅	—	Generated	The table kept calculating the generated value after a column was added, and the new column used its default value. The returned rows were (10,11,0) and (20,21,0).
✅	—	Rev	The table accepted two generated columns, a new regular column, and another row. All three rows kept the correct lowercase name, calculated score, and default note.
⚠️		General	The identity-style column was accepted, but it was not marked as an identity column and did not generate a value for the inserted row.
⚠️		Rev	The catalog returned an empty generation_expression for result instead of upper(source). The same query correctly returned result as ALWAYS, and pg_attribute correctly returned attgenerated = s, so the failure is limited to the missing expression text.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟠 Identity columns lose automatic values

• Severity: High
• Description: The identity-style column was accepted, but it was not marked as an identity column and did not generate a value for the inserted row.
• Impact: Applications that use BY DEFAULT identity columns may create rows with blank IDs instead of generated values. This can break key-based workflows and leave newly inserted records unusable.
• Steps to Reproduce:
  1. Create a table with an integer key, a text source column, and an ordinary text column.
  2. Add an integer column with GENERATED BY DEFAULT AS IDENTITY and add arithmetic and function-generated columns.
  3. Inspect information_schema.columns and pg_attribute, then insert a row without providing the generated columns.
  4. Observe that the identity-style column reports is_identity = NO and attidentity is empty, and its row value is blank while the arithmetic and function columns compute correctly.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: In server/ast/column_table_def.go:91-111, hasGeneratedExpr is false for an identity declaration because the parsed identity form has no Computed.Expr. The code then sets computedByDefaultAsIdentity at line 93, but only the computedAsIdentity branch at lines 101-111 constructs the placeholder nextval expression. Consequently, GENERATED BY DEFAULT AS IDENTITY reaches the ColumnDefinition with generated still nil. At lines 118-135 the branch converts the resolved integer type to a serial type, but it does not set a generated expression or an identity marker. The resulting ColumnDefinition at lines 137-151 therefore has GeneratedExpr nil and Stored false. ALTER TABLE ADD COLUMN delegates directly to this converter at server/ast/alter_table.go:258-267, so the same defect applies to the tested schema change. The smallest practical fix is to give the BY DEFAULT identity path the same supported sequence/default representation as the ALWAYS identity path, while preserving its BY DEFAULT insertion semantics, and to ensure the emitted column metadata carries identity behavior rather than merely changing the resolved type to serial.

Evidence Package

• text/BF-BOUND-1/BF-BOUND-1-identity-boundary.txt — 6 KB

🟡 Generated expression is missing from column metadata

• Severity: Medium
• Description: The catalog returned an empty generation_expression for result instead of upper(source). The same query correctly returned result as ALWAYS, and pg_attribute correctly returned attgenerated = s, so the failure is limited to the missing expression text.
• Impact: Tools and users that inspect database metadata cannot see the expression used by a generated column. The column still works, but schema discovery and migration checks may show incomplete information.
• Steps to Reproduce:
  1. Create generated_catalog with id, source, and result TEXT GENERATED ALWAYS AS (upper(source)) STORED.
  2. Add an integer column named extra with DEFAULT 7.
  3. Query information_schema.columns for generated_catalog and select column_name, is_generated, and generation_expression.
  4. Compare the result column's generation_expression with the expected expression upper(source).
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The runtime result is explained by production code in server/tables/information_schema/columns_table.go. getRowFromColumn sets isGenerated to ALWAYS when col.Generated is non-nil at lines 135 and 143-145, which accounts for the correct marker. However, the row returned for information_schema.columns passes nil for generation_expression at line 196 with an explicit TODO, regardless of col.Generated. That guarantees an empty value for every generated column, including generated_catalog.result, and matches the observed output. The smallest practical fix is to derive a SQL string from col.Generated for this field and return it instead of nil, while preserving nil for ordinary columns. The PR's changed code in server/ast/column_table_def.go only constructs the generated expression and wraps function expressions, and server/ast/select.go only removes synthetic aliases from function arguments; neither file changes information_schema.columns output or the catalog row construction.

Evidence Package

• text/REV-4/REV-4-catalog-metadata-evidence.txt — 3 KB

Tip

Reply with @itoqa to send us feedback on this test run.

[github-actions]

	Main	PR
Total	42090	42090
Successful	19854	19855
Failures	22236	22235
Partial Successes1	5440	5440

	Main	PR
Successful	47.1703%	47.1727%
Failures	52.8297%	52.8273%

${\color{lightgreen}Progressions (1)}$

copyselect

QUERY: drop table test3;

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[coffeegoddd]

@Hydrocharged DOLT

read_tests	from_latency	to_latency	percent_change
covering_index_scan_postgres	2.43	2.43	0.0
groupby_scan_postgres	77.19	78.6	1.83
index_join_postgres	2.26	2.26	0.0
index_join_scan_postgres	1.61	1.61	0.0
index_scan_postgres	467.3	467.3	0.0
oltp_point_select	0.37	0.37	0.0
oltp_read_only	6.32	6.43	1.74
select_random_points	0.72	0.73	1.39
select_random_ranges	1.04	1.04	0.0
table_scan_postgres	467.3	467.3	0.0
types_table_scan_postgres	1170.65	1170.65	0.0

write_tests	from_latency	to_latency	percent_change
oltp_delete_insert_postgres	6.67	6.67	0.0
oltp_insert	3.36	3.36	0.0
oltp_read_write	13.46	13.46	0.0
oltp_update_index	3.55	3.55	0.0
oltp_update_non_index	3.25	3.25	0.0
oltp_write_only	7.04	7.04	0.0
types_delete_insert_postgres	7.17	7.17	0.0

[zachmu]

LGTM but see comment about test

[zachmu]

Good test, but also add a test case that exactly reproduces the issue as reported by the customer.

Also try to find a better home for this test case. issues_test.go is just a random grab bag of regression tests and should be the destination of last resort for truly random one-off issues we can't think of a better place for. This belongs either with tests of alter table, or generated expressions.

[itoqa]

History reset (rebase or force-push detected). Starting test narrative over.

Commit: 6775f7c: 11 test cases ran, 11 passed ✅.

Summary

The run covers database-generated values across schema changes, inserts, updates, key and constraint changes, column removal, expression reconstruction, retrying invalid changes, and repeated operations. It also exercises null handling, nested and equivalent expressions, text and boolean results, aggregation, and edge cases around unsupported expressions and duplicate changes, with all exercised behaviors remaining correct.

Safe to merge — the exercised schema-change, generated-value, expression, and retry behaviors showed no regressions or PR-attributable failures. No merge blocker was identified; overall risk is low.

Tests run by Ito

Result	Severity	Type	Description
✅	—	Alteration	Adding a column, adding a unique constraint, dropping an unrelated column, inserting rows, and updating a base value all succeeded. The arithmetic, uppercase text, and nested absolute-value results stayed correct.
✅	—	General	An invalid generated expression did not leave a broken table behind. After the expression was corrected, the schema change completed and later inserts and updates returned the corrected generated values.
✅	—	General	The generated text value stayed correct after the table key was added, and inserting 7 returned 7. The expression ran without an unwanted argument alias.
✅	—	General	The direct and list-style expressions both survived the key change, returned 5 for an input of 4, and matched each other. The definition display was blank in this build, but both expressions executed successfully without an unwanted alias.
✅	—	General	Dropping an unrelated column did not break the generated value. The later constraint, insert, update, and read all returned the expected results.
✅	—	General	Adding the primary key worked, and repeating the same change returned the expected existing-key error. A later row still received the correct generated value, and the table kept its primary key.
✅	—	Coalesce	The table accepted the primary key change and a later insert. The generated values were 2 for a = 1 and 3 for a = 2.
✅	—	Rev	The table accepted the primary key change and a later insert. The generated column returned true for the inserted row.
✅	—	Rev	The table was created, both rows were inserted, and adding the primary key succeeded. The generated values were 4 for -4 and 7 for 7, with no expression or alias error.
✅	—	Rev	Scalar queries returned the expected values for NULL and 1, and the aggregate returned {0,1} without errors or synthetic aliases.
✅	—	Serialization	The generated column stayed valid after the table changed, and later inserts returned the correct values 2 and 3. No unintended argument alias appeared during the operation.

Tip

Reply with @itoqa to send us feedback on this test run.