Skip to content
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.

Sign up for GitHub

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

Jump to bottom

REGR: fix roundtripping datetimes with sqlite type detection #55690

Merged
merged 2 commits into from
Oct 26, 2023
Merged

REGR: fix roundtripping datetimes with sqlite type detection #55690

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8e29ea
REGR: fix roundtripping datetimes with sqlite type detection
jorisvandenbossche Oct 25, 2023
33c341d
Update pandas/tests/io/test_sql.py
jorisvandenbossche Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.1.2.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Fixed regressions
- Fixed regression in :meth:`DataFrame.sort_index` which was not sorting correctly when the index was a sliced :class:`MultiIndex` (:issue:`55379`)
- Fixed performance regression with wide DataFrames, typically involving methods where all columns were accessed individually (:issue:`55256`, :issue:`55245`)
- Fixed regression in :func:`merge_asof` raising ``TypeError`` for ``by`` with datetime and timedelta dtypes (:issue:`55453`)
- Fixed regression in :meth:`DataFrame.to_sql` not roundtripping datetime columns correctly for sqlite when using ``detect_types`` (:issue:`55554`)

.. ---------------------------------------------------------------------------
.. _whatsnew_212.bug_fixes:
Expand Down
6 changes: 2 additions & 4 deletions pandas/io/sql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2094,19 +2094,17 @@ def _adapt_time(t) -> str:
# Python 3.12+ doesn't auto-register adapters for us anymore

adapt_date_iso = lambda val: val.isoformat()
adapt_datetime_iso = lambda val: val.isoformat()
adapt_datetime_iso = lambda val: val.isoformat(" ")

sqlite3.register_adapter(time, _adapt_time)

sqlite3.register_adapter(date, adapt_date_iso)
sqlite3.register_adapter(datetime, adapt_datetime_iso)

convert_date = lambda val: date.fromisoformat(val.decode())
convert_datetime = lambda val: datetime.fromisoformat(val.decode())
convert_timestamp = lambda val: datetime.fromtimestamp(int(val))
convert_timestamp = lambda val: datetime.fromisoformat(val.decode())
Copy link
Member Author

Choose a reason for hiding this comment

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

@lithomas1 I think you added those from the documentation, but the actual implementation in cpython that we inherited is different: https://github.com/erlend-aasland/cpython/blob/07988dc731bc1e0f6ce2aac201d52e4b78618877/Lib/sqlite3/dbapi2.py#L71-L87

I didn't add that implementation exactly, as I think it should essentially be the same as parsing with fromisoformat (I suppose that didn't exist yet when this was originally implemented in cpython)

Copy link
Member

Choose a reason for hiding this comment

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

Super sorry about this.

I just copied from https://docs.python.org/3/library/sqlite3.html#sqlite3-adapter-converter-recipes blindly when I was doing the changes for Python 3.12, and didn't pay super close attention since tests didn't fail.

Unfortunately, looks like my oversight bit us twice :( .

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, not having test coverage doesn't help, and I have to say that the python docs are also quite misleading because it doesn't seem to work by copying verbatim


sqlite3.register_converter("date", convert_date)
sqlite3.register_converter("datetime", convert_datetime)
sqlite3.register_converter("timestamp", convert_timestamp)

def sql_schema(self) -> str:
Expand Down
19 changes: 19 additions & 0 deletions pandas/tests/io/test_sql.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3333,6 +3333,25 @@ def test_roundtripping_datetimes(sqlite_sqlalchemy_memory_engine):
assert result == "2020-12-31 12:00:00.000000"


@pytest.fixture
def sqlite_builtin_detect_types():
with contextlib.closing(
sqlite3.connect(":memory:", detect_types=sqlite3.PARSE_DECLTYPES)
) as closing_conn:
create_and_load_iris_view(closing_conn)
jorisvandenbossche marked this conversation as resolved.
Show resolved Hide resolved
with closing_conn as conn:
yield conn


def test_roundtripping_datetimes_detect_types(sqlite_builtin_detect_types):
# https://github.com/pandas-dev/pandas/issues/55554
conn = sqlite_builtin_detect_types
df = DataFrame({"t": [datetime(2020, 12, 31, 12)]}, dtype="datetime64[ns]")
df.to_sql("test", conn, if_exists="replace", index=False)
result = pd.read_sql("select * from test", conn).iloc[0, 0]
assert result == Timestamp("2020-12-31 12:00:00.000000")


@pytest.mark.db
def test_psycopg2_schema_support(postgresql_psycopg2_engine):
conn = postgresql_psycopg2_engine
Expand Down