lidavidm commented on code in PR #955:
URL: https://github.com/apache/arrow-adbc/pull/955#discussion_r1281152137


##########
python/adbc_driver_sqlite/tests/test_dbapi.py:
##########
@@ -57,3 +58,25 @@ def test_autocommit(tmp_path: Path) -> None:
         assert not conn._autocommit
         with conn.cursor() as cur:
             cur.executescript("PRAGMA journal_mode = WAL")
+
+
+def test_create_types(tmp_path: Path) -> None:
+    db = tmp_path / "foo.sqlite"
+    with dbapi.connect(f"file:{db}") as conn:
+        tbl = pa.Table.from_pydict({'numbers': [1, 2], 'letters': ['a', 'b']})
+        type_mapping = {
+            'int64': 'INTEGER', 
+            'string': 'TEXT'
+        }
+        expected_types = [type_mapping[str(field.type)] for field in 
tbl.schema]
+
+        with conn.cursor() as cur:
+            cur.adbc_ingest("foo", tbl, "create")
+            cur.execute("PRAGMA table_info('foo')")
+            table_info = cur.fetchall()
+
+            assert len(table_info) == len(tbl.columns)
+            for col in table_info:
+                assert len(col) == 6

Review Comment:
   Explain the constants?



##########
python/adbc_driver_sqlite/tests/test_dbapi.py:
##########
@@ -57,3 +58,25 @@ def test_autocommit(tmp_path: Path) -> None:
         assert not conn._autocommit
         with conn.cursor() as cur:
             cur.executescript("PRAGMA journal_mode = WAL")
+
+
+def test_create_types(tmp_path: Path) -> None:
+    db = tmp_path / "foo.sqlite"
+    with dbapi.connect(f"file:{db}") as conn:
+        tbl = pa.Table.from_pydict({'numbers': [1, 2], 'letters': ['a', 'b']})
+        type_mapping = {
+            'int64': 'INTEGER', 
+            'string': 'TEXT'
+        }
+        expected_types = [type_mapping[str(field.type)] for field in 
tbl.schema]

Review Comment:
   Just hardcode the expected types since it's only two fields?



##########
c/driver/sqlite/sqlite_test.cc:
##########
@@ -190,10 +190,10 @@ class SqliteStatementTest : public ::testing::Test,
   void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpTest()); }
   void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
 
-  void TestSqlIngestUInt64() {
-    std::vector<std::optional<uint64_t>> values = {std::nullopt, 0, INT64_MAX};
-    return TestSqlIngestType(NANOARROW_TYPE_UINT64, values);
-  }
+  void TestSqlIngestUInt8() { GTEST_SKIP() << "Not implemented"; }
+  void TestSqlIngestUInt16() { GTEST_SKIP() << "Not implemented"; }
+  void TestSqlIngestUInt32() { GTEST_SKIP() << "Not implemented"; }
+  void TestSqlIngestUInt64() { GTEST_SKIP() << "Not implemented"; }

Review Comment:
   Why can't we let these be `INTEGER` in the database?



-- 
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]

Reply via email to