Skip to content

GH-48055: [C++][FlightRPC] Allow spaces while parsing Table Type in ODBC #48056

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.

Sign up for GitHub

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

Merged
lidavidm merged 3 commits into from
Nov 9, 2025
Merged

GH-48055: [C++][FlightRPC] Allow spaces while parsing Table Type in ODBC #48056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d8e3d8
Allow spaces while parsing Table Type
alinaliBQ Nov 4, 2025
a6b3cd0
Apply Justin's suggestion
alinaliBQ Nov 5, 2025
bb458f0
Use std::move
justing-bq Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 22 additions & 25 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_tables.cc
View file
Open in desktop

@lidavidm lidavidm Nov 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems reasonable but is there maybe a way to do this with string_view instead of copying the string?

@lidavidm lidavidm Nov 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose it's not likely to matter

@justing-bq justing-bq Nov 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A string copy is still necessary for the sake of trimming the string.

Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,21 @@
#include "arrow/flight/sql/odbc/odbc_impl/record_batch_transformer.h"
#include "arrow/flight/sql/odbc/odbc_impl/util.h"
#include "arrow/flight/types.h"
#include "arrow/util/string.h"

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

using arrow::Result;

static void AddTableType(std::string& table_type, std::vector<std::string>& table_types) {
std::string trimmed_type = arrow::internal::TrimString(table_type);

// Only put the string if the trimmed result is non-empty
if (!trimmed_type.empty()) {
table_types.emplace_back(std::move(trimmed_type));
}
}

void ParseTableTypes(const std::string& table_type,
std::vector<std::string>& table_types) {
bool encountered = false; // for checking if there is a single quote
Expand All @@ -36,36 +46,23 @@ void ParseTableTypes(const std::string& table_type,
for (char temp : table_type) { // while still in the string
switch (temp) { // switch depending on the character
case '\'': // if the character is a single quote
if (encountered) {
encountered = false; // if we already found a single quote, reset encountered
} else {
encountered =
true; // if we haven't found a single quote, set encountered to true
}
// track when we've encountered a single opening quote
// and are still looking for the closing quote
encountered = !encountered;
break;
case ',': // if it is a comma
if (!encountered) { // if we have not found a single quote
table_types.push_back(curr_parse); // put our current string into our vector
curr_parse = ""; // reset the current string
case ',': // if it is a comma
if (!encountered) { // if we have not found a single quote
AddTableType(curr_parse, table_types); // put current string into vector
curr_parse = ""; // reset the current string
break;
}
default: // if it is a normal character
if (encountered && isspace(temp)) {
curr_parse.push_back(temp); // if we have found a single quote put the
// whitespace, we don't care
} else if (temp == '\'' || temp == ' ') {
break; // if the current character is a single quote, trash it and go to
// the next character.
} else {
curr_parse.push_back(temp); // if all of the above failed, put the
// character into the current string
}
break; // go to the next character
[[fallthrough]];
default: // if it is a normal character
curr_parse.push_back(temp); // put the character into the current string
break; // go to the next character
}
}
table_types.emplace_back(
curr_parse); // if we have found a single quote put the whitespace,
// we don't care
AddTableType(curr_parse, table_types);
}

std::shared_ptr<ResultSet> GetTablesForSQLAllCatalogs(
Expand Down
13 changes: 13 additions & 0 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/parse_table_types_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,17 @@ TEST(TableTypeParser, ParsingWithSingleQuotesWithoutLeadingWhiteSpace) {
TEST(TableTypeParser, ParsingWithCommaInsideSingleQuotes) {
AssertParseTest("'TABLE, TEST', 'VIEW, TEMPORARY'", {"TABLE, TEST", "VIEW, TEMPORARY"});
}

TEST(TableTypeParser, ParsingWithManyLeadingAndTrailingWhiteSpaces) {
AssertParseTest(" TABLE , VIEW ", {"TABLE", "VIEW"});
}

TEST(TableTypeParser, ParsingWithOnlyWhiteSpaceBetweenCommas) {
AssertParseTest("TABLE, ,VIEW", {"TABLE", "VIEW"});
}

TEST(TableTypeParser, ParsingWithWhiteSpaceInsideValue) {
AssertParseTest("BASE TABLE", {"BASE TABLE"});
}

} // namespace arrow::flight::sql::odbc
Loading