Fix rowcount not reported for SELECT INTO

[RuchaSK1]

Description

SELECT INTO should report (N rows affected) to the client, but Babelfish was not displaying it despite the rows being inserted correctly into the new table (@@rowcount returned the correct value internally).

The issue is in the TDS DONE token sent after SELECT INTO completes. The DONE token contains a CurCmd field that tells the client what type of statement was executed. Babelfish was sending CurCmd=0xC1 (TDS_CMD_SELECT) for SELECT INTO. The correct value is 0xC2, which was confirmed by capturing TDS traffic with tcpdump and inspecting in Wireshark. With CurCmd=0xC2, clients correctly display the row count.

The fix adds TDS_CMD_SELECTINTO (0xC2) and uses it when the statement is a top-level SELECT INTO which is identified by commandTag==CMDTAG_SELECT with need_to_push_result=false (no rows sent to client), excluding select-assign and INTO-variable cases.

Issues Resolved

Task: #3203
Authored-by: Rucha Kulkarni ruchask@amazon.com
Signed-off-by: Rucha Kulkarni ruchask@amazon.com

Test Scenarios Covered

• Use case based - Yes

• Boundary conditions - Yes

• Arbitrary inputs -

• Negative test cases -

• Minor version upgrade tests -

• Major version upgrade tests -

• Performance tests -

• Tooling impact -

• Client tests -

Check List

• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[RuchaSK1]

/code-review

[kuntalghosh]

AI Code Review

Verdict: APPROVE

Based on the provided code changes, here are my feedback and suggestions:

This PR correctly fixes the TDS DONE token CurCmd field for SELECT INTO statements. The root cause analysis is sound: the client needs CurCmd=0xC2 (SELECT INTO) rather than 0xC1 (SELECT) to display (N rows affected). The implementation cleanly distinguishes SELECT INTO from plain SELECT, select-assign, and INTO-variable cases using existing struct fields.

The C code change is minimal and well-placed. The four-way condition (toplevel && !need_to_push_result && !is_tsql_select_assign_stmt && !into) correctly identifies the SELECT INTO case without false positives. The new TDS_CMD_SELECTINTO constant (0xC2) is inserted in sorted order among the existing TDS command defines and does not collide with any other constant.

Security review of the C/H changes found 0 issues — the change is post-auth only, involves no buffer operations, no SQL construction, no catalog access, and the constant fits trivially in the uint16_t wire field.

Advisory 1 — In test/JDBC/input/select_into_rowcount-vu-verify.sql: consider adding a SET NOCOUNT ON test:

-- Verify @@ROWCOUNT is correct after SELECT INTO
SELECT * INTO #select_into_rowcount_vu_verify_t12 FROM select_into_rowcount_vu_prepare_src;
SELECT @@ROWCOUNT;
GO

The test suite covers multiple rows, single row, zero rows, temp tables, permanent tables, specific columns, and subqueries — good coverage. However, since the NOCOUNT ON interaction is a natural consequence of this change (0xC2 != 0xC1 means the DONE token is now suppressed under SET NOCOUNT ON, which is correct behavior), a test verifying SET NOCOUNT ON; SELECT * INTO ... ; SELECT @@ROWCOUNT still returns the right internal value while suppressing the wire message would strengthen confidence. This is advisory — the behavior is correct by design and the existing test infrastructure implicitly validates it through other tests.

Advisory 2 — In test/JDBC/expected/select_into_rowcount-vu-verify.out: zero-row case has no ROW COUNT line:

-- Zero rows into permanent table
SELECT * INTO select_into_rowcount_vu_verify_t3 FROM select_into_rowcount_vu_prepare_src WHERE 1 = 0;
GO

This is correct — when zero rows are inserted, no (0 rows affected) message is expected on the wire, matching the target system's behavior. Just noting this is intentional and verified.

Notes

• The PR links to GitHub issue #3203 rather than a BABEL JIRA ticket. Since this is an open-source GitHub issue in the babelfish-for-postgresql org, this satisfies the issues-resolved requirement.
• The .out file changes across ~40 existing test files are consistent — each adds a ~~ROW COUNT: N~~ line after SELECT INTO statements, reflecting the newly-reported row count. This is expected behavioral output from the fix.
• Upgrade schedule files correctly include the new select_into_rowcount test across all version upgrade paths.
• Target branch BABEL_6_X_DEV is correct for new feature work.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.

[forestkeeper]

Need to fix the tests first