Fix returning if input is a parameter #28414

gridnevvvit · 2025-11-07T19:19:08Z

Changelog entry

Changelog category

Not for changelog (changelog entry is not required)

Description for reviewers

fix issue #27021

github-actions · 2025-11-07T19:22:16Z

🟢 2025-11-07 19:23:15 UTC The validation of the Pull Request description is successful.

github-actions · 2025-11-07T19:22:40Z

⚪ 2025-11-07 19:22:39 UTC Pre-commit check linux-x86_64-relwithdebinfo for 2d10c18 has started.
⚪ 2025-11-07 19:22:57 UTC Artifacts will be uploaded here
⚪ 2025-11-07 19:25:23 UTC ya make is running...
🟡 2025-11-07 20:53:11 UTC Some tests failed, follow the links below. Going to retry failed tests...

Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
38769	35955	0	1	2791	22

⚪ 2025-11-07 20:53:34 UTC ya make is running... (failed tests rerun, try 2)
🟢 2025-11-07 21:05:58 UTC Tests successful.

Ya make output | Test bloat | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
43 (only retried tests)	28	0	0	0	15

🟢 2025-11-07 21:06:09 UTC Build successful.
🟢 2025-11-07 21:06:29 UTC ydbd size 2.3 GiB changed* by +2.8 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash	main: `371d924`	merge: `2d10c18`	diff	diff %
ydbd size	2 431 882 792 Bytes	2 431 885 672 Bytes	+2.8 KiB	+0.000%
ydbd stripped size	516 543 408 Bytes	516 543 792 Bytes	+384 Bytes	+0.000%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

Copilot

Pull Request Overview

This PR fixes issue #27021 where RETURNING clauses fail when used with UPSERT/DELETE operations on tables with only NOT NULL fields when the data comes from query parameters of type List (via AS_TABLE).

Adds handling for TCoParameter nodes in the returning optimization logic to prevent incorrect wrapping
Introduces comprehensive test coverage for UPSERT and DELETE with RETURNING using parameterized List data
Tests both NOT NULL-only and nullable column scenarios to ensure the fix works correctly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
ydb/core/kqp/opt/physical/effects/kqp_opt_phy_returning.cpp	Adds `TCoParameter` check to prevent wrapping already-precomputed parameter inputs in `TDqPrecompute` nodes for both UPSERT and DELETE operations with RETURNING
ydb/core/kqp/ut/opt/kqp_returning_ut.cpp	Adds helper function `ExecuteReturningQueryWithParams` and two comprehensive test cases validating the fix for UPSERT/DELETE with RETURNING using AS_TABLE with List parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-07T19:22:39Z

ydb/core/kqp/ut/opt/kqp_returning_ut.cpp

+
+        auto deleteParams = deleteParamsBuilder.Build();
+
+        // This should succeed, but currently fails with infinite loop error


This comment is misleading. Since this PR adds a fix for the issue (by handling TCoParameter in the optimization code), the test should now succeed. Consider updating the comment to reflect that this test validates the fix works correctly, e.g., "This should succeed with the fix for issue #27021".

Suggested change

// This should succeed, but currently fails with infinite loop error

// This should succeed with the fix for issue #27021

Copilot · 2025-11-07T19:22:40Z

ydb/core/kqp/ut/opt/kqp_returning_ut.cpp

+        if (QueryService) {
+            auto qdb = kikimr.GetQueryClient();
+            auto qSession = qdb.GetSession().GetValueSync().GetSession();
+            auto settings = NYdb::NQuery::TExecuteQuerySettings()
+                .Syntax(NYdb::NQuery::ESyntax::YqlV1);
+            auto insertResult = qSession.ExecuteQuery(
+                insertQuery, NYdb::NQuery::TTxControl::BeginTx().CommitTx(), insertParams, settings).ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(insertResult.GetStatus(), EStatus::SUCCESS, insertResult.GetIssues().ToString());
+        } else {
+            auto db = kikimr.GetTableClient();
+            auto session = db.CreateSession().GetValueSync().GetSession();
+            auto insertResult = session.ExecuteDataQuery(insertQuery, TTxControl::BeginTx().CommitTx(), insertParams).ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(insertResult.GetStatus(), EStatus::SUCCESS, insertResult.GetIssues().ToString());
+        }


This code block is duplicated in both test functions (lines 804-817 and 926-939). Consider extracting this logic into a helper function similar to ExecuteReturningQueryWithParams to reduce code duplication and improve maintainability.

Copilot · 2025-11-07T19:22:40Z

ydb/core/kqp/ut/opt/kqp_returning_ut.cpp

+        if (QueryService) {
+            auto qdb = kikimr.GetQueryClient();
+            auto qSession = qdb.GetSession().GetValueSync().GetSession();
+            auto settings = NYdb::NQuery::TExecuteQuerySettings()
+                .Syntax(NYdb::NQuery::ESyntax::YqlV1);
+            auto insertResult = qSession.ExecuteQuery(
+                insertQuery, NYdb::NQuery::TTxControl::BeginTx().CommitTx(), insertParams, settings).ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(insertResult.GetStatus(), EStatus::SUCCESS, insertResult.GetIssues().ToString());
+        } else {
+            auto db = kikimr.GetTableClient();
+            auto session = db.CreateSession().GetValueSync().GetSession();
+            auto insertResult = session.ExecuteDataQuery(insertQuery, TTxControl::BeginTx().CommitTx(), insertParams).ExtractValueSync();
+            UNIT_ASSERT_VALUES_EQUAL_C(insertResult.GetStatus(), EStatus::SUCCESS, insertResult.GetIssues().ToString());
+        }


This code block duplicates the logic from lines 804-817. Consider extracting this logic into a helper function similar to ExecuteReturningQueryWithParams to reduce code duplication and improve maintainability.

Copilot · 2025-11-07T19:22:40Z

ydb/core/kqp/ut/opt/kqp_returning_ut.cpp

+
+        auto params = paramsBuilder.Build();
+
+        // This should succeed, but currently fails with infinite loop error


This comment is misleading. Since this PR adds a fix for the issue (by handling TCoParameter in the optimization code), the test should now succeed. Consider updating the comment to reflect that this test validates the fix works correctly, e.g., "This should succeed with the fix for issue #27021".

Suggested change

// This should succeed, but currently fails with infinite loop error

// This should succeed with the fix for issue #27021

github-actions · 2025-11-07T19:23:04Z

⚪ 2025-11-07 19:23:03 UTC Pre-commit check linux-x86_64-release-asan for 2d10c18 has started.
⚪ 2025-11-07 19:23:22 UTC Artifacts will be uploaded here
⚪ 2025-11-07 19:25:41 UTC ya make is running...
🟡 2025-11-07 21:31:28 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Ya make output | Test bloat

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
15460	14747	0	416	263	34

🟢 2025-11-07 21:31:41 UTC Build successful.
🟢 2025-11-07 21:32:10 UTC ydbd size 3.8 GiB changed* by +10.7 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash	main: `371d924`	merge: `2d10c18`	diff	diff %
ydbd size	4 073 133 088 Bytes	4 073 144 048 Bytes	+10.7 KiB	+0.000%
ydbd stripped size	1 511 887 336 Bytes	1 511 890 344 Bytes	+2.9 KiB	+0.000%

^{*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation}

gridnevvvit added 3 commits November 7, 2025 20:00

implement the regression tests

0924d7e

implement a fix and fix the test

4032e74

add more tests

ee8c231

Copilot AI review requested due to automatic review settings November 7, 2025 19:19

gridnevvvit requested a review from a team as a code owner November 7, 2025 19:19

github-actions bot added not-for-changelog and removed not-for-changelog labels Nov 7, 2025

Copilot AI reviewed Nov 7, 2025

View reviewed changes

dahbka-lis approved these changes Nov 7, 2025

View reviewed changes

gridnevvvit merged commit dff4634 into ydb-platform:main Nov 7, 2025
22 checks passed

gridnevvvit added a commit to gridnevvvit/ydb that referenced this pull request Nov 11, 2025

Fix returning if input is a parameter (ydb-platform#28414)

acd19fd

gridnevvvit added a commit to gridnevvvit/ydb that referenced this pull request Nov 11, 2025

Fix returning if input is a parameter (ydb-platform#28414)

3c2a6f3

gridnevvvit added a commit that referenced this pull request Nov 12, 2025

Fix returning if input is a parameter (#28414) (#28672)

8f9bd3e

gridnevvvit added a commit that referenced this pull request Nov 12, 2025

Fix returning if input is a parameter (#28414) (#28673)

ab3c1f6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix returning if input is a parameter #28414

Fix returning if input is a parameter #28414

Uh oh!

gridnevvvit commented Nov 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 7, 2025

Uh oh!

Copilot AI Nov 7, 2025

Uh oh!

Copilot AI Nov 7, 2025

Uh oh!

Copilot AI Nov 7, 2025

Uh oh!

github-actions bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		auto deleteParams = deleteParamsBuilder.Build();

		// This should succeed, but currently fails with infinite loop error

	// This should succeed, but currently fails with infinite loop error
	// This should succeed with the fix for issue #27021


		auto params = paramsBuilder.Build();

		// This should succeed, but currently fails with infinite loop error

Fix returning if input is a parameter #28414

Fix returning if input is a parameter #28414

Uh oh!

Conversation

gridnevvvit commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog entry

Changelog category

Description for reviewers

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gridnevvvit commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading