alinaliBQ commented on code in PR #50021:
URL: https://github.com/apache/arrow/pull/50021#discussion_r3730479540
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -758,32 +753,35 @@ 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);
+ // SQLite (the mock backend) does not support schema DDL.
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
+}
- EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
- << GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
+TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoDropSchema) {
+ SQLUINTEGER value;
+ GetInfo(this->conn, SQL_DROP_SCHEMA, &value);
+
+ // The Dremio backend does not report schema DDL support either.
+ EXPECT_EQ(static_cast<SQLUINTEGER>(0), 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) {
+ SQLUINTEGER value;
+ GetInfo(this->conn, SQL_DROP_TABLE, &value);
+ EXPECT_EQ(static_cast<SQLUINTEGER>(SQL_DT_DROP_TABLE), value);
+}
+
+TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoDropTable) {
SQLUINTEGER value;
GetInfo(this->conn, SQL_DROP_TABLE, &value);
+ // The Dremio backend does not report table DDL support.
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
Review Comment:
I raised https://github.com/apache/arrow/issues/50828 for this
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -758,32 +753,35 @@ 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:
Can we change this to a `ConnectionInfoHandleTest` since the mock test and
remote test are testing the same attribute?
```suggestion
TEST_F(ConnectionInfoHandleTest, TestSQLGetInfoDropSchema) {
```
##########
cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc:
##########
@@ -758,32 +753,35 @@ 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);
+ // SQLite (the mock backend) does not support schema DDL.
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
+}
- EXPECT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn))
- << GetOdbcErrorMessage(SQL_HANDLE_DBC, this->conn);
+TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoDropSchema) {
+ SQLUINTEGER value;
+ GetInfo(this->conn, SQL_DROP_SCHEMA, &value);
+
+ // The Dremio backend does not report schema DDL support either.
+ EXPECT_EQ(static_cast<SQLUINTEGER>(0), 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) {
+ SQLUINTEGER value;
+ GetInfo(this->conn, SQL_DROP_TABLE, &value);
+ EXPECT_EQ(static_cast<SQLUINTEGER>(SQL_DT_DROP_TABLE), value);
+}
+
+TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoDropTable) {
SQLUINTEGER value;
GetInfo(this->conn, SQL_DROP_TABLE, &value);
+ // The Dremio backend does not report table DDL support.
EXPECT_EQ(static_cast<SQLUINTEGER>(0), value);
Review Comment:
Out of scope of this PR but helpful to mention, there is a gap that 2 tests
are missing from the original implementation:
- `TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoCreateTable)` for
`SQL_CREATE_TABLE`
- `TEST_F(ConnectionInfoRemoteTest, TestSQLGetInfoCreateSchema)` for
`SQL_CREATE_SCHEMA`
Theoretically these tests should also return `0` to signal table and schema
DDL is not supported in Dremio instance. It also explains why we were only
seeing inconsistencies in `SQL_DROP_TABLE` and `SQL_DROP_SCHEMA`.
--
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]
