Skip to content

BUG: to_sql with datetime.time values with sqlite #11547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.18.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ Bug Fixes

- Bug in ``.plot`` potentially modifying the ``colors`` input when the number of columns didn't match the number of series provided (:issue:`12039`).
- Bug in ``Series.plot`` failing when index has a ``CustomBusinessDay`` frequency (:issue:`7222`).

- Bug in ``.to_sql`` for ``datetime.time`` values with sqlite fallback (:issue:`8341`)
- Bug in ``read_excel`` failing to read data with one column when ``squeeze=True`` (:issue:`12157`)
- Bug in ``.groupby`` where a ``KeyError`` was not raised for a wrong column if there was only one row in the dataframe (:issue:`11741`)
- Bug in ``.read_csv`` with dtype specified on empty data producing an error (:issue:`12048`)
Expand Down
11 changes: 10 additions & 1 deletion pandas/io/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"""

from __future__ import print_function, division
from datetime import datetime, date
from datetime import datetime, date, time

import warnings
import traceback
Expand Down Expand Up @@ -1403,6 +1403,15 @@ class SQLiteTable(SQLTable):
Instead of a table variable just use the Create Table statement.
"""

def __init__(self, *args, **kwargs):
# GH 8341
# register an adapter callable for datetime.time object
import sqlite3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this standard practice to do this in the init? (rather than at a class level)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is to only have this adapter being registered if you actually use pandas with to_sql, not if you just import pandas to do other things (as this changes the behaviour of sqlite).

But actually, we should maybe test how this possibly interferes with sqlalchemy. @nbonnotte Can you test that writing/reading time values with sqlalchemy still works after having run this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nbonnotte did you for @jorisvandenbossche comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, I'm a bit busy right now, but I've noted :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done... I think, see the test below

@jorisvandenbossche is it what you had in mind?

# this will transform time(12,34,56,789) into '12:34:56.000789'
# (this is what sqlalchemy does)
sqlite3.register_adapter(time, lambda _: _.strftime("%H:%M:%S.%f"))
super(SQLiteTable, self).__init__(*args, **kwargs)

def sql_schema(self):
return str(";\n".join(self.table))

Expand Down
26 changes: 22 additions & 4 deletions pandas/io/tests/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,22 @@ def test_datetime_time(self):
res = read_sql_table('test_time', self.conn)
tm.assert_frame_equal(res, df)

# GH8341
# first, use the fallback to have the sqlite adapter put in place
sqlite_conn = TestSQLiteFallback.connect()
sql.to_sql(df, "test_time2", sqlite_conn, index=False)
res = sql.read_sql_query("SELECT * FROM test_time2", sqlite_conn)
ref = df.applymap(lambda _: _.strftime("%H:%M:%S.%f"))
tm.assert_frame_equal(ref, res) # check if adapter is in place
# then test if sqlalchemy is unaffected by the sqlite adapter
sql.to_sql(df, "test_time3", self.conn, index=False)
if self.flavor == 'sqlite':
res = sql.read_sql_query("SELECT * FROM test_time3", self.conn)
ref = df.applymap(lambda _: _.strftime("%H:%M:%S.%f"))
tm.assert_frame_equal(ref, res)
res = sql.read_sql_table("test_time3", self.conn)
tm.assert_frame_equal(df, res)

def test_mixed_dtype_insert(self):
# see GH6509
s1 = Series(2**25 + 1, dtype=np.int32)
Expand Down Expand Up @@ -1957,12 +1973,14 @@ def test_datetime_date(self):
tm.assert_frame_equal(res, df)

def test_datetime_time(self):
# test support for datetime.time
# test support for datetime.time, GH #8341
df = DataFrame([time(9, 0, 0), time(9, 1, 30)], columns=["a"])
# test it raises an error and not fails silently (GH8341)
df.to_sql('test_time', self.conn, index=False, flavor=self.flavor)
res = read_sql_query('SELECT * FROM test_time', self.conn)
if self.flavor == 'sqlite':
self.assertRaises(sqlite3.InterfaceError, sql.to_sql, df,
'test_time', self.conn)
# comes back as strings
expected = df.applymap(lambda _: _.strftime("%H:%M:%S.%f"))
tm.assert_frame_equal(res, expected)

def _get_index_columns(self, tbl_name):
ixs = sql.read_sql_query(
Expand Down