Fixed Issue #3328

[Hydrocharged]

Fixes #3328.

Stacked on #3351.

[itoqa]

Commit: b1b96c0: 7 test cases ran, 1 failed ❌, 6 passed ✅.

Summary

The run covered generated-column metadata across ordinary and generated fields, default and nullability handling, expression grouping and normalization, and consistency across client access paths. It included normal usage and a focused edge case involving quoted punctuation in an expression; the overall behavior was healthy aside from a minor metadata-formatting issue.

Safe to merge — the only PR-attributable issue is a minor edge-case defect in how generated-expression metadata is normalized, while generated-column behavior and the broader metadata contract remain intact. The issue affects display of metadata rather than execution, so it is a caveat rather than a merge blocker.

Tests run by Ito

Result	Severity	Type	Description
❌		Rev	The generated column is correctly marked as ALWAYS and the quoted closing parenthesis is preserved, but the metadata reports ((("label" || ')'))) instead of the normalized expression "label" || ')'.
✅	—	General	Generated columns kept the grouping in the arithmetic expression. The grouped value was 8 and the ungrouped value was 5, as expected.
✅	—	General	The direct and nested PostgreSQL-compatible clients returned the same generated-column metadata, including stable quoting and preserved nested grouping.
✅	—	General	Ordinary columns kept their existing metadata in a table that also contained generated columns. The mixed table matched the control table with zero mismatches, and all three generated columns reported generated metadata.
✅	—	Columns	Ordinary columns keep their existing metadata when a table also has a generated column.
✅	—	Expression	The metadata query marks generated columns as ALWAYS and shows each stored expression. Ordinary columns remain NEVER with no generated expression.
✅	—	Rev	The default value remains under column_default, while only the stored generated column is marked ALWAYS and receives a generation expression.

Tip

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

[itoqa]

Quoted parentheses keep extra expression wrappers

What failed: The generated column is correctly marked as ALWAYS and the quoted closing parenthesis is preserved, but the metadata reports ((("label" || ')'))) instead of the normalized expression "label" || ')'.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug

• Severity: Minor
• Impact: Users and tools that inspect generated-column metadata may see extra parentheses around expressions containing a quoted parenthesis. The generated column still works, and the issue is limited to this metadata display case.
• Steps to Reproduce:
  1. Create a table with a text column generated from label || ')'.
  2. Query information_schema.columns for that generated column.
  3. Compare generation_expression with the expression without the redundant outer wrapper.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The PR changes getRowFromColumn in server/tables/information_schema/columns_table.go at lines 152-155 to pass every generated expression through trimEnclosingParens. The helper at lines 205-210 removes an outer pair only when parenDepthStaysPositive returns true. The new balance check at lines 213-228 scans every rune without tracking SQL quote state. For the expression produced by the failing case, the ) inside the single-quoted literal is treated as a structural closing parenthesis, causing the depth to become negative and making the helper leave the redundant outer parentheses in place. The PR diff explicitly adds this implementation in the production path, while the test result confirms the resulting metadata value. The smallest practical fix is to make the balance scan ignore parentheses inside quoted SQL strings, or to use the parser's expression structure when deciding which wrappers enclose the full expression.
• Why this is likely a bug: The expected metadata format removes only parentheses that wrap the entire generated expression while preserving parentheses that are part of the expression or a quoted value. The reported result violates that contract for a valid SQL expression containing ')', and the production helper explains the mismatch directly: it performs character-level balancing without recognizing quotes. This is a real, reproducible metadata normalization defect rather than a setup issue; a targeted quote-aware balance check fixes the failing case without changing generated-column execution.

Relevant code

server/tables/information_schema/columns_table.go:151-155

columnDefault := information_schema.GetColumnDefault(ctx, col.Default)
var generationExpression any
if col.Generated != nil {
	generationExpression = trimEnclosingParens(col.Generated.String())
}

server/tables/information_schema/columns_table.go:205-228

func trimEnclosingParens(expr string) string {
	for len(expr) > 1 && expr[0] == '(' && expr[len(expr)-1] == ')' && parenDepthStaysPositive(expr[1:len(expr)-1]) {
		expr = expr[1 : len(expr)-1]
	}
	return expr
}

func parenDepthStaysPositive(expr string) bool {
	depth := 0
	for _, r := range expr {
		switch r {
		case '(':
			depth++
		case ')':
			depth--
			if depth < 0 {
				return false
			}
		}
	}
	return true
}

Evidence Package

• text/REV-1/REV-1-metadata-and-code-analysis.txt — 3 KB

Copy prompt for an agent

Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Minor severity — Quoted parentheses keep extra expression wrappers**

**What failed:** The generated column is correctly marked as ALWAYS and the quoted closing parenthesis is preserved, but the metadata reports `((("label" || ')')))` instead of the normalized expression `"label" || ')'`.

- **Impact:** Users and tools that inspect generated-column metadata may see extra parentheses around expressions containing a quoted parenthesis. The generated column still works, and the issue is limited to this metadata display case.
- **Steps to reproduce:**
  1. Create a table with a text column generated from `label || ')'`.
  2. Query `information_schema.columns` for that generated column.
  3. Compare `generation_expression` with the expression without the redundant outer wrapper.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR changes `getRowFromColumn` in `server/tables/information_schema/columns_table.go` at lines 152-155 to pass every generated expression through `trimEnclosingParens`. The helper at lines 205-210 removes an outer pair only when `parenDepthStaysPositive` returns true. The new balance check at lines 213-228 scans every rune without tracking SQL quote state. For the expression produced by the failing case, the `)` inside the single-quoted literal is treated as a structural closing parenthesis, causing the depth to become negative and making the helper leave the redundant outer parentheses in place. The PR diff explicitly adds this implementation in the production path, while the test result confirms the resulting metadata value. The smallest practical fix is to make the balance scan ignore parentheses inside quoted SQL strings, or to use the parser's expression structure when deciding which wrappers enclose the full expression.
- **Why this is likely a bug:** The expected metadata format removes only parentheses that wrap the entire generated expression while preserving parentheses that are part of the expression or a quoted value. The reported result violates that contract for a valid SQL expression containing `')'`, and the production helper explains the mismatch directly: it performs character-level balancing without recognizing quotes. This is a real, reproducible metadata normalization defect rather than a setup issue; a targeted quote-aware balance check fixes the failing case without changing generated-column execution.

**Relevant code:**

`server/tables/information_schema/columns_table.go:151-155`

~~~go
columnDefault := information_schema.GetColumnDefault(ctx, col.Default)
var generationExpression any
if col.Generated != nil {
	generationExpression = trimEnclosingParens(col.Generated.String())
}
~~~

`server/tables/information_schema/columns_table.go:205-228`

~~~go
func trimEnclosingParens(expr string) string {
	for len(expr) > 1 && expr[0] == '(' && expr[len(expr)-1] == ')' && parenDepthStaysPositive(expr[1:len(expr)-1]) {
		expr = expr[1 : len(expr)-1]
	}
	return expr
}

func parenDepthStaysPositive(expr string) bool {
	depth := 0
	for _, r := range expr {
		switch r {
		case '(':
			depth++
		case ')':
			depth--
			if depth < 0 {
				return false
			}
		}
	}
	return true
}
~~~

[github-actions]

	Main	PR
Total	42090	42090
Successful	19855	19865
Failures	22235	22225
Partial Successes1	5440	5441

	Main	PR
Successful	47.1727%	47.1965%
Failures	52.8273%	52.8035%

${\color{red}Regressions (1)}$

subselect

QUERY:          select count(*) from tenk1 t
where (exists(select 1 from tenk1 k where k.unique1 = t.unique2) or ten < 0);
RECEIVED ERROR: timeout during Receive

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

aggregates

QUERY: insert into bytea_test_table values(decode('ff','hex'));

QUERY: insert into bytea_test_table values(decode('aa','hex'));

constraints

QUERY: SELECT * FROM DEFAULTEXPR_TBL;

strings

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: insert into toasttest values(decode(repeat('1234567890',10000),'escape'));

QUERY: SELECT decode('1234567890abcdef00', 'hex');

QUERY: SELECT decode(encode(('\x' || repeat('1234567890abcdef0001', 7))::bytea,
                     'base64'), 'base64');

QUERY: SELECT decode(encode('\x1234567890abcdef00', 'escape'), 'escape');

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	77.19	0.0
index_join_postgres	2.22	2.22	0.0
index_join_scan_postgres	1.61	1.61	0.0
index_scan_postgres	467.3	458.96	-1.78
oltp_point_select	0.36	0.37	2.78
oltp_read_only	6.43	6.43	0.0
select_random_points	0.72	0.72	0.0
select_random_ranges	1.04	1.03	-0.96
table_scan_postgres	467.3	458.96	-1.78
types_table_scan_postgres	1170.65	1149.76	-1.78

write_tests	from_latency	to_latency	percent_change
oltp_delete_insert_postgres	6.67	6.67	0.0
oltp_insert	3.3	3.3	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	6.91	-1.85
types_delete_insert_postgres	7.17	7.17	0.0

[zachmu]

Change looks fine, see comments.

[zachmu]

Put this in information_schema test or similar

[zachmu]

Might need to change after changes further down the stack. We don't really enforce the right level of parens in our serialized default / generated column expressions, it's been very tricky to get right. All we really care about is having at least one enclosing pair.

[zachmu]

Does Dolt have this same bug? Please confirm and file if so.