Skip to content

Commit 4576192

Browse files
GH-48055: [C++][FlightRPC] Allow spaces while parsing Table Type in ODBC (#48056)
### Rationale for this change Allow spaces while parsing Table Type in ODBC. For example, driver should be able to interpret `" TABLE , VIEW "` as "TABLE" and "VIEW". ### What changes are included in this PR? - Allow spaces while parsing Table Type in ODBC & Add 3 tests ### Are these changes tested? Tested locally on Windows MSVC. ### Are there any user-facing changes? N/A * GitHub Issue: #48055 Lead-authored-by: Alina (Xi) Li <[email protected]> Co-authored-by: justing-bq <[email protected]> Co-authored-by: justing-bq <[email protected]> Signed-off-by: David Li <[email protected]>
1 parent bbb3d48 commit 4576192

File tree

2 files changed

+35
-25
lines changed

2 files changed

+35
-25
lines changed

cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_statement_get_tables.cc

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,21 @@
2323
#include "arrow/flight/sql/odbc/odbc_impl/record_batch_transformer.h"
2424
#include "arrow/flight/sql/odbc/odbc_impl/util.h"
2525
#include "arrow/flight/types.h"
26+
#include "arrow/util/string.h"
2627

2728
namespace arrow::flight::sql::odbc {
2829

2930
using arrow::Result;
3031

32+
static void AddTableType(std::string& table_type, std::vector<std::string>& table_types) {
33+
std::string trimmed_type = arrow::internal::TrimString(table_type);
34+
35+
// Only put the string if the trimmed result is non-empty
36+
if (!trimmed_type.empty()) {
37+
table_types.emplace_back(std::move(trimmed_type));
38+
}
39+
}
40+
3141
void ParseTableTypes(const std::string& table_type,
3242
std::vector<std::string>& table_types) {
3343
bool encountered = false; // for checking if there is a single quote
@@ -36,36 +46,23 @@ void ParseTableTypes(const std::string& table_type,
3646
for (char temp : table_type) { // while still in the string
3747
switch (temp) { // switch depending on the character
3848
case '\'': // if the character is a single quote
39-
if (encountered) {
40-
encountered = false; // if we already found a single quote, reset encountered
41-
} else {
42-
encountered =
43-
true; // if we haven't found a single quote, set encountered to true
44-
}
49+
// track when we've encountered a single opening quote
50+
// and are still looking for the closing quote
51+
encountered = !encountered;
4552
break;
46-
case ',': // if it is a comma
47-
if (!encountered) { // if we have not found a single quote
48-
table_types.push_back(curr_parse); // put our current string into our vector
49-
curr_parse = ""; // reset the current string
53+
case ',': // if it is a comma
54+
if (!encountered) { // if we have not found a single quote
55+
AddTableType(curr_parse, table_types); // put current string into vector
56+
curr_parse = ""; // reset the current string
5057
break;
5158
}
52-
default: // if it is a normal character
53-
if (encountered && isspace(temp)) {
54-
curr_parse.push_back(temp); // if we have found a single quote put the
55-
// whitespace, we don't care
56-
} else if (temp == '\'' || temp == ' ') {
57-
break; // if the current character is a single quote, trash it and go to
58-
// the next character.
59-
} else {
60-
curr_parse.push_back(temp); // if all of the above failed, put the
61-
// character into the current string
62-
}
63-
break; // go to the next character
59+
[[fallthrough]];
60+
default: // if it is a normal character
61+
curr_parse.push_back(temp); // put the character into the current string
62+
break; // go to the next character
6463
}
6564
}
66-
table_types.emplace_back(
67-
curr_parse); // if we have found a single quote put the whitespace,
68-
// we don't care
65+
AddTableType(curr_parse, table_types);
6966
}
7067

7168
std::shared_ptr<ResultSet> GetTablesForSQLAllCatalogs(

cpp/src/arrow/flight/sql/odbc/odbc_impl/parse_table_types_test.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,17 @@ TEST(TableTypeParser, ParsingWithSingleQuotesWithoutLeadingWhiteSpace) {
4949
TEST(TableTypeParser, ParsingWithCommaInsideSingleQuotes) {
5050
AssertParseTest("'TABLE, TEST', 'VIEW, TEMPORARY'", {"TABLE, TEST", "VIEW, TEMPORARY"});
5151
}
52+
53+
TEST(TableTypeParser, ParsingWithManyLeadingAndTrailingWhiteSpaces) {
54+
AssertParseTest(" TABLE , VIEW ", {"TABLE", "VIEW"});
55+
}
56+
57+
TEST(TableTypeParser, ParsingWithOnlyWhiteSpaceBetweenCommas) {
58+
AssertParseTest("TABLE, ,VIEW", {"TABLE", "VIEW"});
59+
}
60+
61+
TEST(TableTypeParser, ParsingWithWhiteSpaceInsideValue) {
62+
AssertParseTest("BASE TABLE", {"BASE TABLE"});
63+
}
64+
5265
} // namespace arrow::flight::sql::odbc

0 commit comments

Comments
 (0)