alinaliBQ commented on code in PR #50021:
URL: https://github.com/apache/arrow/pull/50021#discussion_r3299307571
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -752,32 +745,18 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDropDomain) {
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
}
-TYPED_TEST(ConnectionInfoHandleTest, TestSQLGetInfoDropSchema) {
- // GH-49482 TODO: resolve inconsitent return value for SQL_DROP_SCHEMA and
change test
- // type to `ConnectionInfoTest`
- this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TEST_F(ConnectionInfoMockTest, TestSQLGetInfoDropSchema) {
Review Comment:
Could you use `TYPED_TEST(ConnectionInfoTest,` here to be consistent with
rest of code base?
##########
cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc:
##########
@@ -494,22 +496,22 @@ bool GetInfoCache::LoadInfoFromServer() {
}
case SqlInfoOptions::SQL_TRANSACTIONS_SUPPORTED: {
transactions_supported =
-
reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;
Review Comment:
Please fix the CI build failures from `checked_cast` in this file:
https://github.com/apache/arrow/actions/runs/26406439067/job/77730895486?pr=50021#step:7:4419
```
/Users/runner/work/arrow/arrow/cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc:499:19:
error: use of undeclared identifier 'checked_cast'; did you mean
'internal::checked_cast'?
499 |
checked_cast<BooleanScalar*>(scalar->child_value().get())->value;
| ^~~~~~~~~~~~
| internal::checked_cast
/Users/runner/work/arrow/arrow/cpp/src/arrow/util/checked_cast.h:28:19:
note: 'internal::checked_cast' declared here
```
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -616,18 +616,11 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoAlterTable) {
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
}
-TYPED_TEST(ConnectionInfoHandleTest, TestSQLGetInfoCatalogLocation) {
- // GH-49482 TODO: resolve inconsitent return value for SQL_CATALOG_LOCATION
and change
- // test type to `ConnectionInfoTest`
- this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoCatalogLocation) {
Review Comment:
Could you use `TYPED_TEST(ConnectionInfoTest,` here to be consistent with
rest of code base?
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
Review Comment:
@vanshaj2023 To ensure maximum coverage, please build and run the ODBC tests
locally against a Dremio docker instance with documentation here:
https://github.com/apache/arrow/blob/cd1811be25ce2c485ad00f3be0bf0ac44ea7b423/cpp/src/arrow/flight/sql/odbc/README.md?plain=1#L88-L89
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -752,32 +745,18 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDropDomain) {
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
}
-TYPED_TEST(ConnectionInfoHandleTest, TestSQLGetInfoDropSchema) {
- // GH-49482 TODO: resolve inconsitent return value for SQL_DROP_SCHEMA and
change test
- // type to `ConnectionInfoTest`
- this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TEST_F(ConnectionInfoMockTest, TestSQLGetInfoDropSchema) {
SQLUINTEGER value;
GetInfo(this->conn, SQL_DROP_SCHEMA, &value);
- EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
-
- EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
- << GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
+ EXPECT_EQ(static_cast<SQLUINTEGER>(SQL_DS_DROP_SCHEMA), value);
}
-TYPED_TEST(ConnectionInfoHandleTest, TestSQLGetInfoDropTable) {
- // GH-49482 TODO: resolve inconsitent return value for SQL_DROP_TABLE and
change test
- // type to `ConnectionInfoTest`
- this->ConnectWithString(this->GetConnectionString(), this->conn);
-
+TEST_F(ConnectionInfoMockTest, TestSQLGetInfoDropTable) {
Review Comment:
Could you use `TYPED_TEST(ConnectionInfoTest,` here to be consistent with
rest of code base?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
