-
Notifications
You must be signed in to change notification settings - Fork 3.9k
GH-47711: [C++][FlightRPC] Enable ODBC query execution #48032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
| statement->Prepare(query); | ||
| statement->ExecutePrepared(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on removing boost as part of #48067
|
@lidavidm this draft PR is ready for review! Please have a look |
Co-Authored-By: alinalibq <[email protected]>
97c94b0 to
8625cfa
Compare
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