fix(c/driver): return NOT_FOUND for GetTableSchema

lidavidm · Sep 1, 2023 · 1f26859 · 1f26859
1 parent 932b721
commit 1f26859
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 3 deletions.
diff --git a/c/driver/postgresql/connection.cc b/c/driver/postgresql/connection.cc
@@ -1308,7 +1308,12 @@ AdbcStatusCode PostgresConnection::GetTableSchema(const char* catalog,
   StringBuilderReset(&query);
 
   RAISE_ADBC(result_helper.Prepare());
-  RAISE_ADBC(result_helper.Execute());
+  auto result = result_helper.Execute();
+  if (result != ADBC_STATUS_OK) {
+    if (std::string(error->sqlstate, 5) == "42P01") {
+      return ADBC_STATUS_NOT_FOUND;
+    }
+  }
 
   auto uschema = nanoarrow::UniqueSchema();
   ArrowSchemaInit(uschema.get());

diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc
@@ -651,6 +651,7 @@ int TupleReader::GetNext(struct ArrowArray* out) {
   struct ArrowArray tmp;
   NANOARROW_RETURN_NOT_OK(BuildOutput(&tmp, &error));
 
+  PQclear(result_);
   // Check the server-side response
   result_ = PQgetResult(conn_);
   const ExecStatusType pq_status = PQresultStatus(result_);

diff --git a/c/driver/sqlite/sqlite.c b/c/driver/sqlite/sqlite.c
@@ -877,6 +877,7 @@ AdbcStatusCode SqliteConnectionGetTableSchema(struct AdbcConnection* connection,
     return ADBC_STATUS_INTERNAL;
   }
 
+  // TODO(apache/arrow-adbc#1025): escape
   if (StringBuilderAppend(&query, "%s%s", "SELECT * FROM ", table_name) != 0) {
     StringBuilderReset(&query);
     SetError(error, "[SQLite] Call to StringBuilderAppend failed");
@@ -888,8 +889,8 @@ AdbcStatusCode SqliteConnectionGetTableSchema(struct AdbcConnection* connection,
       sqlite3_prepare_v2(conn->conn, query.buffer, query.size, &stmt, /*pzTail=*/NULL);
   StringBuilderReset(&query);
   if (rc != SQLITE_OK) {
-    SetError(error, "[SQLite] Failed to prepare query: %s", sqlite3_errmsg(conn->conn));
-    return ADBC_STATUS_INTERNAL;
+    SetError(error, "[SQLite] GetTableSchema: %s", sqlite3_errmsg(conn->conn));
+    return ADBC_STATUS_NOT_FOUND;
   }
 
   struct ArrowArrayStream stream = {0};

diff --git a/c/integration/duckdb/duckdb_test.cc b/c/integration/duckdb/duckdb_test.cc
@@ -75,6 +75,7 @@ class DuckDbConnectionTest : public ::testing::Test,
 
   void TestAutocommitDefault() { GTEST_SKIP(); }
   void TestMetadataGetTableSchema() { GTEST_SKIP(); }
+  void TestMetadataGetTableSchemaNotFound() { GTEST_SKIP(); }
   void TestMetadataGetTableTypes() { GTEST_SKIP(); }
 
  protected:

diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc
@@ -430,6 +430,19 @@ void ConnectionTest::TestMetadataGetTableSchema() {
                                     {"strings", NANOARROW_TYPE_STRING, NULLABLE}}));
 }
 
+void ConnectionTest::TestMetadataGetTableSchemaNotFound() {
+  ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+  ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+  ASSERT_THAT(quirks()->DropTable(&connection, "thistabledoesnotexist", &error),
+              IsOkStatus(&error));
+
+  Handle<ArrowSchema> schema;
+  ASSERT_THAT(AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr,
+                                           /*db_schema=*/nullptr, "thistabledoesnotexist",
+                                           &schema.value, &error),
+              IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+}
+
 void ConnectionTest::TestMetadataGetTableTypes() {
   ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
   ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));

diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h
@@ -188,6 +188,7 @@ class ConnectionTest {
 
   void TestMetadataGetInfo();
   void TestMetadataGetTableSchema();
+  void TestMetadataGetTableSchemaNotFound();
   void TestMetadataGetTableTypes();
 
   void TestMetadataGetObjectsCatalogs();
@@ -219,6 +220,9 @@ class ConnectionTest {
   TEST_F(FIXTURE, MetadataCurrentDbSchema) { TestMetadataCurrentDbSchema(); }           \
   TEST_F(FIXTURE, MetadataGetInfo) { TestMetadataGetInfo(); }                           \
   TEST_F(FIXTURE, MetadataGetTableSchema) { TestMetadataGetTableSchema(); }             \
+  TEST_F(FIXTURE, MetadataGetTableSchemaNotFound) {                                     \
+    TestMetadataGetTableSchemaNotFound();                                               \
+  }                                                                                     \
   TEST_F(FIXTURE, MetadataGetTableTypes) { TestMetadataGetTableTypes(); }               \
   TEST_F(FIXTURE, MetadataGetObjectsCatalogs) { TestMetadataGetObjectsCatalogs(); }     \
   TEST_F(FIXTURE, MetadataGetObjectsDbSchemas) { TestMetadataGetObjectsDbSchemas(); }   \

diff --git a/go/adbc/driver/snowflake/driver.go b/go/adbc/driver/snowflake/driver.go
@@ -163,6 +163,10 @@ func errToAdbcErr(code adbc.Status, err error) error {
 		var sqlstate [5]byte
 		copy(sqlstate[:], []byte(sferr.SQLState))
 
+		if sferr.SQLState == "42S02" {
+			code = adbc.StatusNotFound
+		}
+
 		return adbc.Error{
 			Code:       code,
 			Msg:        sferr.Error(),