GH-47711: [C++][FlightRPC] Enable ODBC query execution#48032
GH-47711: [C++][FlightRPC] Enable ODBC query execution#48032lidavidm merged 1 commit intoapache:mainfrom
Conversation
|
|
| statement->Prepare(query); | ||
| statement->ExecutePrepared(); |
There was a problem hiding this comment.
we implemented SQLExecDirect to use prepare/execute to use prepared statements to align with the behavior with JDBC
72e8db0 to
97c94b0
Compare
| ASSERT_EQ(SQL_SUCCESS, | ||
| SQLExecDirect(this->stmt, &sql0[0], static_cast<SQLINTEGER>(sql0.size()))); | ||
|
|
||
| // GH-47713 TODO: Uncomment call to SQLFetch SQLGetData after implementation |
There was a problem hiding this comment.
I thought we weren't expecting these tests to pass yet? Are there build issues with this not commented out?
There was a problem hiding this comment.
When this PR is just created as draft, we aren't expecting them to pass. But once the pre-requisite ODBC API PRs get merged in main, I am expecting this PR to pass the CI tests after a rebase. And some calls to unimplemented API (SQLFetch and SQLGetData) are commented out because they will be enabled in a different PR
| return boost::make_optional(it != attribute_.end(), it->second); | ||
| } | ||
|
|
||
| boost::optional<std::shared_ptr<ResultSetMetadata>> FlightSqlStatement::Prepare( |
|
@lidavidm this draft PR is ready for review! Please have a look |
97c94b0 to
8625cfa
Compare
8625cfa to
96c85a3
Compare
|
PR is rebased and ready for review |
|
It appears it needs to be rebased again |
96c85a3 to
74544d8
Compare
|
@lidavidm Yup, that might be a trend for these ODBC API PRs 😂 . I have rebased the PR and tested locally on MSVC |
Co-Authored-By: alinalibq <alina.li@improving.com>
74544d8 to
22d48eb
Compare
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit b2e8f25. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Rationale for this change
Enable query execution in ODBC.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?
N/A