Skip to content

Conversation

@alinaliBQ
Copy link
Contributor

@alinaliBQ alinaliBQ commented Nov 3, 2025

Rationale for this change

Implement SQLMoreResults for fetching more results. Arrow protocol doesn't support multiple result sets, so no data is returned by default in SQLMoreResults.

What changes are included in this PR?

  • SQLMoreResults & tests

Are these changes tested?

Tested on local MSVC

Are there any user-facing changes?

N/A

class StatementMockTest : public FlightSQLODBCMockTestBase {};
class StatementRemoteTest : public FlightSQLODBCRemoteTestBase {};
using TestTypes = ::testing::Types<StatementMockTest, StatementRemoteTest>;
TYPED_TEST_SUITE(StatementTest, TestTypes);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 tests TestSQLMoreResultsNoData and TestSQLMoreResultsInvalidFunctionSequence are added in this PR.

@alinaliBQ alinaliBQ changed the title GH-47713: [C++][FlightRPC] WIP: ODBC SQLMoreResults implementation GH-47713: [C++][FlightRPC] ODBC SQLMoreResults implementation Nov 10, 2025
@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Nov 10, 2025
@alinaliBQ
Copy link
Contributor Author

@lidavidm this draft PR is ready for review! Please have a look

@alinaliBQ alinaliBQ force-pushed the gh-47713-sql-more-results branch 2 times, most recently from 5f90811 to e841e88 Compare December 4, 2025 22:29
@alinaliBQ alinaliBQ marked this pull request as ready for review December 4, 2025 22:29
@alinaliBQ alinaliBQ requested a review from lidavidm as a code owner December 4, 2025 22:29
Nit updates

Co-Authored-By: alinalibq <[email protected]>
@alinaliBQ alinaliBQ force-pushed the gh-47713-sql-more-results branch from e841e88 to f1a2076 Compare December 13, 2025 00:04
@lidavidm lidavidm merged commit f8e40da into apache:main Dec 13, 2025
41 of 45 checks passed
@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Dec 13, 2025
@lidavidm
Copy link
Member

@alinaliBQ one note for the future, do you mind making one issue per PR? It seems several of these PRs were connected to one issue, which isn't a big deal, but we generally try to have one issue per PR. Organizationally you could make a top-level issue and use GitHub's sub issues if that helps keep track of things. Thanks!

@alinaliBQ
Copy link
Contributor Author

@lidavidm Yes I can have 1 issue per PR in future. Your suggestion of using a top-level issue makes sense, just that I have tried that before and I don't think I have access since I am not a maintainer. I will mention parent issues in the child issue tickets.

@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit f8e40da.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants