Skip to content

Fix max1Row error for set-returning functions over point lookups - #3151

Open
zachmu wants to merge 1 commit into
mainfrom
zachmu/issue3111
Open

Fix max1Row error for set-returning functions over point lookups#3151
zachmu wants to merge 1 commit into
mainfrom
zachmu/issue3111

Conversation

@zachmu

@zachmu zachmu commented Aug 20, 2026

Copy link
Copy Markdown
Member

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.

GMS sets QFlagMax1Row when a query is a strict unique-index point lookup,
and the DoltgresHandler uses that flag to take a result-spooling shortcut
that errors if more than one row is produced. That analysis doesn't account
for set-returning functions (unnest, generate_series, etc.) in the
projection, which multiply output rows:

    SELECT unnest(indkey) FROM pg_index WHERE indexrelid = 'x'::regclass;
    ERROR: result max1Row iterator returned more than one row

Adds a new analyzer rule, UnsetMax1RowForSRFs, to the OnceAfterAll batch
that unsets QFlagMax1Row whenever the plan contains an expression
implementing sql.RowIterExpression that returns a row iterator (the
representation of SRFs in Doltgres, e.g. CompiledFunction.IsSRF).

Only the simple query protocol takes the max1Row shortcut, so the
regression tests include wire-level tests using pgproto3.Query in addition
to the extended-protocol ScriptTests.

Fixes #3111
@github-actions

Copy link
Copy Markdown
Contributor
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

itoqa Bot commented Aug 20, 2026

Copy link
Copy Markdown

Ito QA test results
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

View full run

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.
⚠️ Medium severity 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 Medium severity
  • 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

Tip

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

@coffeegoddd

Copy link
Copy Markdown
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unnest and other set-returning functions incompatible with max1Row iterator

2 participants