Skip to content

Conversation

@alinaliBQ
Copy link
Contributor

@alinaliBQ alinaliBQ commented Oct 29, 2025

Rationale for this change

GH-46584

What changes are included in this PR?

Iterate over endpoints to get data

Are these changes tested?

Tested in CI and locally on Windows MSVC. Test flight_sql_stream_chunk_buffer_test.cc is added

Are there any user-facing changes?

N/A

@github-actions
Copy link

⚠️ GitHub issue #46584 has been automatically assigned in GitHub to PR creator.

@alinaliBQ alinaliBQ force-pushed the gh-46584-iterate-endpoint branch from 64e84f5 to 488ff8e Compare October 29, 2025 18:53
@alinaliBQ alinaliBQ marked this pull request as ready for review October 29, 2025 20:21
@alinaliBQ alinaliBQ requested a review from lidavidm as a code owner October 29, 2025 20:21
@lidavidm lidavidm changed the title GH-46584: [C++][FlightRPC] Iterate over endpoint GH-46584: [C++][FlightRPC] Iterate over endpoints in ODBC driver Oct 31, 2025

namespace arrow::flight::sql::odbc {

using arrow::Result;
Copy link
Member

Choose a reason for hiding this comment

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

Can we avoid putting using in headers?

Comment on lines +95 to +104
void VerifyArraysContainIntsOnly(std::shared_ptr<Array> intArray) {
for (int64_t i = 0; i < intArray->length(); ++i) {
// null values are accepted
if (!intArray->IsNull(i)) {
auto scalar_data = intArray->GetScalar(i).ValueOrDie();
std::string scalar_str = ConvertToJson(*scalar_data);
ASSERT_TRUE(std::all_of(scalar_str.begin(), scalar_str.end(), ::isdigit));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just check the array type?

std::unique_ptr<FlightClient> temp_flight_client;
util::ThrowIfNotOK(FlightClient::Connect(endpoint_locations[0], client_options)
.Value(&temp_flight_client));
temp_flight_sql_client.reset(new FlightSqlClient(std::move(temp_flight_client)));
Copy link
Member

Choose a reason for hiding this comment

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

make_shared?

Comment on lines +68 to +70
return boost::make_optional(
is_not_ok || is_not_empty,
std::make_pair(std::move(result), temp_flight_sql_client));
Copy link
Member

Choose a reason for hiding this comment

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

Can we get rid of boost?

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Oct 31, 2025
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