This is an automated email from the ASF dual-hosted git repository. zeroshade pushed a commit to branch fixup-metadata-getobjects-snowflake in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
commit 71dd6bc151cb5ff659f041262d707513bff8230b Author: Matt Topol <[email protected]> AuthorDate: Wed Oct 16 12:25:10 2024 -0400 fix lint and flakey test --- c/driver/snowflake/snowflake_test.cc | 2 +- c/validation/adbc_validation_connection.cc | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/c/driver/snowflake/snowflake_test.cc b/c/driver/snowflake/snowflake_test.cc index 90735b3ff..67d3cbb3f 100644 --- a/c/driver/snowflake/snowflake_test.cc +++ b/c/driver/snowflake/snowflake_test.cc @@ -133,7 +133,7 @@ class SnowflakeQuirks : public adbc_validation::DriverQuirks { case NANOARROW_TYPE_LARGE_STRING: case NANOARROW_TYPE_LIST: case NANOARROW_TYPE_LARGE_LIST: - return NANOARROW_TYPE_STRING; + return NANOARROW_TYPE_STRING; default: return ingest_type; } diff --git a/c/validation/adbc_validation_connection.cc b/c/validation/adbc_validation_connection.cc index 6ef430213..9cef88c6d 100644 --- a/c/validation/adbc_validation_connection.cc +++ b/c/validation/adbc_validation_connection.cc @@ -701,7 +701,6 @@ void ConnectionTest::TestMetadataGetObjectsTablesTypes() { db_schemas_index < ArrowArrayViewListChildOffset(catalog_db_schemas_list, row + 1); db_schemas_index++) { - // db_schema_tables should either be null or an empty list for (int64_t tables_index = ArrowArrayViewListChildOffset(db_schema_tables_list, db_schemas_index); @@ -743,13 +742,12 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { struct TestCase { std::optional<std::string> filter; - std::vector<std::string> column_names; - std::vector<int32_t> ordinal_positions; + std::vector<std::pair<std::string, int32_t>> columns; }; std::vector<TestCase> test_cases; - test_cases.push_back({std::nullopt, {"int64s", "strings"}, {1, 2}}); - test_cases.push_back({"in%", {"int64s"}, {1}}); + test_cases.push_back({std::nullopt, {{"int64s", 1}, {"strings", 2}}}); + test_cases.push_back({"in%", {{"int64s", 1}}}); const std::string catalog = quirks()->catalog(); @@ -759,13 +757,14 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { SCOPED_TRACE(scope); StreamReader reader; + std::vector<std::pair<std::string, int32_t>> columns; std::vector<std::string> column_names; std::vector<int32_t> ordinal_positions; ASSERT_THAT( AdbcConnectionGetObjects( - &connection, ADBC_OBJECT_DEPTH_COLUMNS, catalog.c_str(), nullptr, nullptr, nullptr, - test_case.filter.has_value() ? test_case.filter->c_str() : nullptr, + &connection, ADBC_OBJECT_DEPTH_COLUMNS, catalog.c_str(), nullptr, nullptr, + nullptr, test_case.filter.has_value() ? test_case.filter->c_str() : nullptr, &reader.stream.value, &error), IsOkStatus(&error)); ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); @@ -835,10 +834,9 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { std::string temp(name.data, name.size_bytes); std::transform(temp.begin(), temp.end(), temp.begin(), [](unsigned char c) { return std::tolower(c); }); - column_names.push_back(std::move(temp)); - ordinal_positions.push_back( - static_cast<int32_t>(ArrowArrayViewGetIntUnsafe( - table_columns->children[1], columns_index))); + columns.emplace_back(std::move(temp), + static_cast<int32_t>(ArrowArrayViewGetIntUnsafe( + table_columns->children[1], columns_index))); } } } @@ -848,8 +846,11 @@ void ConnectionTest::TestMetadataGetObjectsColumns() { } while (reader.array->release); ASSERT_TRUE(found_expected_table) << "Did (not) find table in metadata"; - ASSERT_EQ(test_case.column_names, column_names); - ASSERT_EQ(test_case.ordinal_positions, ordinal_positions); + // metadata columns do not guarantee the order they are returned in, we can + // avoid the test being flakey by sorting the column names we found + std::sort(columns.begin(), columns.end(), + [](const auto& a, const auto& b) -> bool { return a.first < b.first; }); + ASSERT_EQ(test_case.columns, columns); } }
