Fix max1Row error for set-returning functions over point lookups

[zachmu]

Fixes a "result max1Row iterator returned more than one row" error when a set-returning function is projected over a unique-index point lookup.

Fixes #3111.

[github-actions]

	Main	PR
Total	42090	42090
Successful	18980	18980
Failures	23110	23110
Partial Successes1	5461	5461

	Main	PR
Successful	45.0938%	45.0938%
Failures	54.9062%	54.9062%

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. ↩

[itoqa]

Commit: e8f72d9: 14 test cases ran, 13 passed ✅, 1 additional finding ⚠️.

Summary

The run broadly exercised database query behavior across ordinary lookups, expanding result sets, empty results, repeated and mixed queries, multiple source rows, and recovery after invalid input. It also covered an unsupported lateral query shape that remains a compatibility gap, while the tested non-lateral paths behaved normally.

Safe to merge — the only observed failure is a medium-severity, pre-existing limitation unrelated to this PR, with no regression or PR-attributable failure identified. The change’s covered query and recovery behaviors remain healthy, so the lateral limitation is a flag for later rather than a merge blocker.

Tests run by Ito

Result	Severity	Type	Description
✅	—	General	An empty array query returned no rows, and the next query returned 1, 2, and 3 successfully.
✅	—	General	The ordinary lookup and the one-row set-returning query each returned one row. The expanding query returned both rows, and the ordinary lookup still worked afterward.
✅	—	General	A malformed set-returning query showed the expected input error, then the same connection and a fresh connection both returned the normal lookup row.
✅	—	General	Catalog index lookup, array lookup, and generated-series queries all returned their complete rows on the same database connection.
✅	—	General	Queries that combine a point lookup with generated values returned every expected row and completed normally. The transformed and direct forms both worked without an iterator error.
✅	—	General	Two identical array-expansion queries returned 10, 20, and 30 each, with no errors or broken connection state.
✅	—	Preserve	The point lookup returned exactly one row with the value 1 and completed successfully.
✅	—	Rev	The scalar query returned 3 and key 1 in one row, and the follow-up lookup returned key 1 normally. Both queries completed successfully on the same connection.
✅	—	Rev	The malformed generate_series query showed the expected input error, and the same connection then returned 10, 20, 30 from the array query and 1 from the health check.
✅	—	Rev	A query over two source rows returned all three array values, 10, 20, and 30, in source-row order without an iterator error.
✅	—	Unnest	The array lookup returned 10, 20, and 30 and finished normally with three result rows.
✅	—	Unnest	The catalog lookup returned both index keys, 1 and 2, and completed successfully.
✅	—	Unnest	The query returned the generated values 1, 2, and 3 and completed successfully.
⚠️		Rev	The table setup succeeds, but the valid query returns unsupported syntax: values row(generate_series(1, 3)) and produces no rows. It should return three rows for the single matching table record.

Additional Findings Details

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

🟡 Lateral queries fail to return generated rows

• Severity: Medium
• Description: The table setup succeeds, but the valid query returns unsupported syntax: values row(generate_series(1, 3)) and produces no rows. It should return three rows for the single matching table record.
• Impact: Queries that use a lateral generated series with a point lookup fail instead of returning the expected rows. Users can still use other query forms, and there is no evidence of data loss or corrupted data.
• Steps to Reproduce:
  1. Create table lateral_rev2 with integer primary key pk and text column marker.
  2. Insert one row with pk = 1.
  3. Run SELECT t.pk, g FROM lateral_rev2 AS t CROSS JOIN LATERAL generate_series(1, 3) AS s(g) WHERE t.pk = 1 through the simple query protocol.
  4. Check the response for rows (1, 1), (1, 2), and (1, 3).
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The PR registers UnsetMax1RowForSRFs in server/analyzer/init.go:118-127 and implements it in server/analyzer/unset_max1row_for_srfs.go:29-45. The rule only clears QFlagMax1Row after transform.InspectExpressions finds an expression implementing sql.RowIterExpression whose ReturnsRowIter() is true. The handler then chooses resultForMax1RowIter only when that flag remains set, at server/doltgres_handler.go:403-411; otherwise it uses h.resultForDefaultIter at lines 412-420. The failing query is a lateral FROM-item, not the projection SRF shape covered by the PR's new tests. The corrected local execution reaches the application and fails after successful setup, so this is not a shell-quoting or missing-fixture failure. The smallest practical fix is to make lateral SRF plan handling recognize and execute the generated rows, or to ensure the lateral plan is lowered into a supported RowIterExpression before the max1Row decision; this should be targeted to the lateral plan path rather than changing ordinary point lookups.

Evidence Package

• text/REV-2/REV-2-lateral-srf-failure.txt — 2 KB

Tip

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

[coffeegoddd]

@zachmu DOLT

read_tests	from_latency_median	to_latency_median	is_faster
covering_index_scan_postgres	2.43	2.48	0
groupby_scan_postgres	75.82	75.82	0
index_join_postgres	2.18	2.22	0
index_join_scan_postgres	1.58	1.58	0
index_scan_postgres	493.24	484.44	0
oltp_point_select	0.36	0.36	0
oltp_read_only	6.32	6.32	0
select_random_points	0.7	0.7	0
select_random_ranges	1.01	1.03	0
table_scan_postgres	484.44	484.44	0
types_table_scan_postgres	1235.62	1235.62	0

write_tests	from_latency_median	to_latency_median	is_faster
bulk_insert	0.001	0.001	0
oltp_delete_insert_postgres	6.67	6.67	0
oltp_insert	3.36	3.3	0
oltp_read_write	13.22	13.22	0
oltp_update_index	3.55	3.55	0
oltp_update_non_index	3.25	3.25	0
oltp_write_only	6.91	6.91	0
types_delete_insert_postgres	7.17	7.17	0

[itoqa]

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

Commit: 3a50c41: 12 test cases ran, 1 failed ❌, 10 passed ✅, 1 additional finding ⚠️.

Summary

Coverage spans normal lookups and array expansion, generated values, index metadata, connection stability, boundary sizes, composite keys, and multi-row result handling. Basic and edge-size behaviors work, but more complex array projections expose incorrect duplicate rows and ordering.

Merge with caution — this PR is associated with a medium-severity wrong-results defect in array projections with companion fields, making affected query results unreliable. A separate multi-row duplication issue is not attributable to this PR and is a flag for later.

Tests run by Ito

Result	Severity	Type	Description
❌		Rev	The query should return three rows with source_id 7 and elem values 303, 202, and 101. Instead, each value appears three times for nine total rows, in ascending order, and the second execution returns the same incorrect result.
✅	—	Array	A primary-key lookup expanded the stored array into three rows: 10, 20, and 30, without an error.
✅	—	Array	The point lookup succeeded and returned the generated values 1, 2, and 3.
✅	—	Array	The index lookup returned both index-column values, 1 and 2, without an error.
✅	—	General	The query returned zero, one, three, and three rows as expected, then the same connection accepted another query.
✅	—	General	The server returned all 5,000 generated rows in order, reported the correct count, and accepted another query on the same connection.
✅	—	Lookup	The point lookup succeeded and returned one row containing 2.
✅	—	Lookup	The scalar lookup returned one row with the value 2, as expected.
✅	—	Protocol	The database returned both index values, reported two rows, and kept the connection ready for another query.
✅	—	Protocol	The local PostgreSQL connection returned all three array values, reported SELECT 3, and ended in the ready state without an error.
✅	—	Rev	A lookup using two key columns returned all three array values, 11, 22, and 33, in the expected order and completed successfully.
⚠️		Rev	The query returned eight rows instead of four: (1,10), (1,20), (1,10), (1,20), (2,30), (2,40), (2,30), and (2,40). The follow-up SELECT 1 succeeded, so the failure is incorrect result cardinality rather than a connection error.

Additional Findings Details

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

🟡 Multi-row lookup duplicates array values

• Severity: Medium
• Description: The query returned eight rows instead of four: (1,10), (1,20), (1,10), (1,20), (2,30), (2,40), (2,30), and (2,40). The follow-up SELECT 1 succeeded, so the failure is incorrect result cardinality rather than a connection error.
• Impact: Queries that use a set-returning function with multiple source rows can return duplicate rows, so users may receive incorrect results. The database session remains usable, but the affected query must be rewritten or corrected before its results can be trusted.
• Steps to Reproduce:
  1. Create a table with an integer primary key and an integer array column.
  2. Insert row 1 with array values 10 and 20, and row 2 with array values 30 and 40.
  3. Run SELECT id, unnest(arr) AS value FROM the_table WHERE id IN (1,2) ORDER BY id, value.
  4. Count the rows and compare each id/value pair with the two input arrays.
  5. Run SELECT 1 on the same connection after the result is returned.
• Stub / mock content: A local-only development authentication bypass was applied so the test client could connect to the database; no application data, query results, or SRF behavior was mocked.
• Code Analysis: The local run demonstrates a deterministic cardinality defect: each of the two source rows is expanded to its two array elements twice. The unnest implementation in server/functions/unnest.go:32-49 creates one iterator over the input array and advances its index once per returned element, so it does not itself contain a second pass or duplicate an element. The projection execution is marked as containing nested iterators by server/analyzer/optimize_functions.go:90-129, which is the existing path responsible for expanding SRF values alongside source rows. The PR adds server/analyzer/unset_max1row_for_srfs.go:29-45 and registers it at server/analyzer/init.go:118-126; that rule only inspects expressions when QFlagMax1Row is set and then unsets that flag. It does not rewrite the Project node, source iterator, or SRF iterator, so the available diff does not establish that the new rule introduced the duplicate expansion. The smallest practical fix is to correct the existing nested-SRF projection iteration so each input source row evaluates and consumes its SRF iterator exactly once, while retaining the PR rule for the separate max-one-row error case.

Evidence Package

• text/REV-4/REV-4-multi-row-srf-output.txt — 2 KB

Tip

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

[itoqa]

SRF results are duplicated and misordered

What failed: The query should return three rows with source_id 7 and elem values 303, 202, and 101. Instead, each value appears three times for nine total rows, in ascending order, and the second execution returns the same incorrect result.

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

• Severity: Medium
• Impact: Queries that expand array values alongside other columns can return duplicate rows and the wrong sort order. Users relying on this query shape may receive silently incorrect data until the query is changed or the defect is fixed.
• Steps to Reproduce:
  1. Create a table with an integer primary key and integer array column.
  2. Insert one row with ID 7 and array values 101, 202, and 303.
  3. Run SELECT id AS source_id, unnest(arr) AS elem FROM the_table WHERE id = 7 ORDER BY elem DESC.
  4. Check the column names, row count, values, and order, then run the same query again in the same session.
• Stub / mock content: Local database authentication was disabled so the test client could connect to the development instance. The table and array data were created specifically for this test; no application response mocks or route interceptions were used.
• Code Analysis: The PR adds server/analyzer/unset_max1row_for_srfs.go and registers UnsetMax1RowForSRFs in server/analyzer/init.go:118-126. The new rule scans expressions in lines 34-40, detects an expression implementing sql.RowIterExpression with ReturnsRowIter() true, and clears QFlagMax1Row at lines 42-44. In server/doltgres_handler.go:392-417, a result with a set QFlagMax1Row uses resultForMax1RowIter, while a cleared flag takes h.resultForDefaultIter. The retest reaches the latter path and returns a three-by-three expansion, so the PR's new flag transition exposes an incorrect SRF projection iteration path when a scalar companion column is present. The requested ORDER BY is also not preserved by that path. The smallest practical fix is to make the default SRF projection iterator consume each source row once and preserve the sort operator's output, or narrowly avoid clearing the flag for this shape until that iterator behavior is fixed; a broad query-engine rewrite is not required.
• Why this is likely a bug: The corrected local query completed twice with valid aliases and stable, reproducible nine-row output, while the fixture contains one source row and three array elements. The duplicate rows are silently wrong rather than a parser or connection failure, and the output contradicts the explicit descending sort requirement. The PR directly changes which handler branch runs by clearing QFlagMax1Row, so this is a production-code defect in the newly enabled SRF execution path rather than an authentication setup artifact.

Relevant code

server/analyzer/unset_max1row_for_srfs.go:34-44

containsSRF := false
transform.InspectExpressions(ctx, node, func(ctx *sql.Context, expr sql.Expression) bool {
	if rowIterExpr, ok := expr.(sql.RowIterExpression); ok && rowIterExpr.ReturnsRowIter() {
		containsSRF = true
	}
	return !containsSRF
})
if containsSRF {
	qFlags.Unset(sql.QFlagMax1Row)
}

server/analyzer/init.go:118-126

analyzer.OnceAfterAll = insertAnalyzerRules(analyzer.OnceAfterAll, analyzer.QuoteDefaultColumnValueNamesId, false,
		analyzer.Rule{Id: ruleId_OptimizeFunctions, Apply: OptimizeFunctions},
		...
		analyzer.Rule{Id: ruleId_UnsetMax1RowForSRFs, Apply: UnsetMax1RowForSRFs},
	)

server/doltgres_handler.go:403-417

} else if analyzer.FlagIsSet(qFlags, sql.QFlagMax1Row) {
	...
	r, err = resultForMax1RowIter(...)
} else {
	...
	r, processedAtLeastOneBatch, err = h.resultForDefaultIter(...)
}

Evidence Package

• text/REV-3/REV-3-srf-duplicate-order.txt — 2 KB

Copy prompt for an agent

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

**Medium severity — SRF results are duplicated and misordered**

**What failed:** The query should return three rows with source_id 7 and elem values 303, 202, and 101. Instead, each value appears three times for nine total rows, in ascending order, and the second execution returns the same incorrect result.

- **Impact:** Queries that expand array values alongside other columns can return duplicate rows and the wrong sort order. Users relying on this query shape may receive silently incorrect data until the query is changed or the defect is fixed.
- **Steps to reproduce:**
  1. Create a table with an integer primary key and integer array column.
  2. Insert one row with ID 7 and array values 101, 202, and 303.
  3. Run SELECT id AS source_id, unnest(arr) AS elem FROM the_table WHERE id = 7 ORDER BY elem DESC.
  4. Check the column names, row count, values, and order, then run the same query again in the same session.
- **Stub / mock content:** Local database authentication was disabled so the test client could connect to the development instance. The table and array data were created specifically for this test; no application response mocks or route interceptions were used.
- **Code analysis:** The PR adds server/analyzer/unset_max1row_for_srfs.go and registers UnsetMax1RowForSRFs in server/analyzer/init.go:118-126. The new rule scans expressions in lines 34-40, detects an expression implementing sql.RowIterExpression with ReturnsRowIter() true, and clears QFlagMax1Row at lines 42-44. In server/doltgres_handler.go:392-417, a result with a set QFlagMax1Row uses resultForMax1RowIter, while a cleared flag takes h.resultForDefaultIter. The retest reaches the latter path and returns a three-by-three expansion, so the PR's new flag transition exposes an incorrect SRF projection iteration path when a scalar companion column is present. The requested ORDER BY is also not preserved by that path. The smallest practical fix is to make the default SRF projection iterator consume each source row once and preserve the sort operator's output, or narrowly avoid clearing the flag for this shape until that iterator behavior is fixed; a broad query-engine rewrite is not required.
- **Why this is likely a bug:** The corrected local query completed twice with valid aliases and stable, reproducible nine-row output, while the fixture contains one source row and three array elements. The duplicate rows are silently wrong rather than a parser or connection failure, and the output contradicts the explicit descending sort requirement. The PR directly changes which handler branch runs by clearing QFlagMax1Row, so this is a production-code defect in the newly enabled SRF execution path rather than an authentication setup artifact.

**Relevant code:**

`server/analyzer/unset_max1row_for_srfs.go:34-44`

~~~go
containsSRF := false
transform.InspectExpressions(ctx, node, func(ctx *sql.Context, expr sql.Expression) bool {
	if rowIterExpr, ok := expr.(sql.RowIterExpression); ok && rowIterExpr.ReturnsRowIter() {
		containsSRF = true
	}
	return !containsSRF
})
if containsSRF {
	qFlags.Unset(sql.QFlagMax1Row)
}
~~~

`server/analyzer/init.go:118-126`

~~~go
analyzer.OnceAfterAll = insertAnalyzerRules(analyzer.OnceAfterAll, analyzer.QuoteDefaultColumnValueNamesId, false,
		analyzer.Rule{Id: ruleId_OptimizeFunctions, Apply: OptimizeFunctions},
		...
		analyzer.Rule{Id: ruleId_UnsetMax1RowForSRFs, Apply: UnsetMax1RowForSRFs},
	)
~~~

`server/doltgres_handler.go:403-417`

~~~go
} else if analyzer.FlagIsSet(qFlags, sql.QFlagMax1Row) {
	...
	r, err = resultForMax1RowIter(...)
} else {
	...
	r, processedAtLeastOneBatch, err = h.resultForDefaultIter(...)
}
~~~

[itoqa]

Ito Diff Report — 3a50c41 → c463a51: 14 test cases ran, 2 fixed ✅, 12 passing ✅.

Diff Summary

Coverage focuses on database lookups and array expansion, including scalar and multi-row results, companion-value alignment, ordering, repeated and concurrent use, retry behavior, connection reuse, and catalog metadata. It includes normal flows plus boundary, protocol-level, concurrency, and regression-oriented checks.

Safe to merge — the exercised behaviors are healthy, with no regressions, new failures, or previously identified PR-attributable failures. The remaining previously passing areas were not exercised in this run but present no merge-blocking signal.

Tests run by Ito

Result	State	Severity	Type	Description
✅	❌->✅ Fixed	—	Rev	The point lookup returned all three array values in order, and each row kept the matching companion value.
✅	❌->✅ Fixed	—	Rev	The unique-key lookup returned all three array values in order, kept the companion data aligned, and completed successfully.
✅	Passing	—	General	The mixed lookup returned three rows with values 10, 20, and 30, while the scalar-only lookup returned one row with pk=1.
✅	Passing	—	General	Both queries returned the full three-value result, 10, 20, and 30, with no missing rows or errors.
✅	Passing	—	General	The array expansion returned 10, 20, and 30, and the catalog index query returned the expected index shape without an iterator error.
✅	Passing	—	General	The query returned three generated rows, completed normally, and the same connection successfully ran a second query.
✅	Passing	—	General	The first and last scalar lookups both returned 1, and the middle query returned all three generated rows.
✅	Passing	—	General	Five pairs of clients ran the same array query at the same time. Every client received 10, 20, and 30 with no error or mixed results.
✅	Passing	—	General	Two clients submitted the same array query at nearly the same time, and both received all three rows: 10, 20, and 30.
✅	Passing	—	General	Sending the same catalog query again before the first response was fully drained produced two separate, complete responses. Both responses included the expected rows, completion tag, and ready state.
✅	Passing	—	Lookup	The point lookup returned exactly one row containing pk=1 and completed without an error.
✅	Passing	—	Projection	The point lookup returned all three array values, 10, 20, and 30, without an error.
✅	Passing	—	Rev	The primary-key and unique-key lookups each returned one matching row with the stored values unchanged.
✅	Passing	—	Wire	The catalog query returned both index values and completed normally. The connection reached the ready state without an error.
⏸️	Skipped	—	Array	A primary-key lookup expanded the stored array into three rows: 10, 20, and 30, without an error.
⏸️	Skipped	—	Array	The point lookup succeeded and returned the generated values 1, 2, and 3.
⏸️	Skipped	—	Array	The index lookup returned both index-column values, 1 and 2, without an error.
⏸️	Skipped	—	General	The query returned zero, one, three, and three rows as expected, then the same connection accepted another query.
⏸️	Skipped	—	General	The server returned all 5,000 generated rows in order, reported the correct count, and accepted another query on the same connection.
⏸️	Skipped	—	Lookup	The point lookup succeeded and returned one row containing 2.
⏸️	Skipped	—	Lookup	The scalar lookup returned one row with the value 2, as expected.
⏸️	Skipped	—	Protocol	The database returned both index values, reported two rows, and kept the connection ready for another query.
⏸️	Skipped	—	Protocol	The local PostgreSQL connection returned all three array values, reported SELECT 3, and ended in the ready state without an error.
⏸️	Skipped	—	Rev	A lookup using two key columns returned all three array values, 11, 22, and 33, in the expected order and completed successfully.

Tip

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