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.

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

Fix BUG: read_sql tries to convert blob/varbinary to string with pyarrow backend #60105

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2244869
Add fix for #59242
kastkeepitjumpinlikekangaroos Oct 25, 2024
bd00fc5
add skip import
kastkeepitjumpinlikekangaroos Oct 25, 2024
6a23f05
address comment
kastkeepitjumpinlikekangaroos Nov 12, 2024
2f261c8
merged main
kastkeepitjumpinlikekangaroos Nov 12, 2024
8f900b8
also fix for dtype_backend=numpy_nullable
kastkeepitjumpinlikekangaroos Nov 13, 2024
536b1ed
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Nov 14, 2024
8965a1d
merge main
kastkeepitjumpinlikekangaroos Nov 17, 2024
a32b4a6
fix
kastkeepitjumpinlikekangaroos Nov 17, 2024
f412c72
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Nov 20, 2024
a0200d0
address comment
kastkeepitjumpinlikekangaroos Nov 20, 2024
10ca030
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Nov 21, 2024
ba2e82d
address comment
kastkeepitjumpinlikekangaroos Dec 4, 2024
8dd45ca
Merge branch 'fix-59242' of github.com:kastkeepitjumpinlikekangaroos/…
kastkeepitjumpinlikekangaroos Dec 4, 2024
602194a
remove cast
kastkeepitjumpinlikekangaroos Dec 4, 2024
f8ae285
fix test
kastkeepitjumpinlikekangaroos Dec 4, 2024
02514e0
fix logic
kastkeepitjumpinlikekangaroos Dec 4, 2024
ef5c2ed
fix
kastkeepitjumpinlikekangaroos Dec 7, 2024
da7817a
fix
kastkeepitjumpinlikekangaroos Dec 9, 2024
de5c604
also include pa.OpaqueType
kastkeepitjumpinlikekangaroos Dec 9, 2024
e025d84
fix
kastkeepitjumpinlikekangaroos Dec 9, 2024
3d83bab
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Dec 9, 2024
6ea5785
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Dec 13, 2024
88accb3
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Dec 24, 2024
c066ebf
fix failing test
kastkeepitjumpinlikekangaroos Dec 27, 2024
2afeed5
Merge branch 'fix-59242' of github.com:kastkeepitjumpinlikekangaroos/…
kastkeepitjumpinlikekangaroos Dec 27, 2024
de286a4
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Dec 27, 2024
a99365a
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Dec 30, 2024
a8f69e5
addressing comments
kastkeepitjumpinlikekangaroos Jan 21, 2025
2f8ac63
Merge branch 'fix-59242' of github.com:kastkeepitjumpinlikekangaroos/…
kastkeepitjumpinlikekangaroos Jan 21, 2025
1a0b2cf
fix
kastkeepitjumpinlikekangaroos Jan 21, 2025
1da6ee4
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Jan 21, 2025
4c1c282
clarify
kastkeepitjumpinlikekangaroos Jan 29, 2025
fe4bdba
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Jan 29, 2025
71d94e5
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Feb 1, 2025
8813f77
Merge branch 'main' into fix-59242
kastkeepitjumpinlikekangaroos Feb 4, 2025
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ MultiIndex
I/O
^^^
- :meth:`DataFrame.to_excel` was storing decimals as strings instead of numbers (:issue:`49598`)
- Bug in :func:`read_sql` causing an unintended exception when byte data was being converted to string when using the pyarrow dtype_backend (:issue:`59242`)
-

Period
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/dtypes/dtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,9 @@ def type(self):
elif pa.types.is_null(pa_type):
# TODO: None? pd.NA? pa.null?
return type(pa_type)
elif isinstance(pa_type, pa.ExtensionType):
elif isinstance(pa_type, pa.ExtensionType) or isinstance(
pa_type, pa.OpaqueType
Copy link
Member

Choose a reason for hiding this comment

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

Hmm AFAIK the OpaqueType is returned when there is no suitable database type, but isn't binary already returned directly? It is a bit unclear what this additional isinstance check is for

Choose a reason for hiding this comment

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

I had to add this because adbc-driver-postgresql is returning an OpaqueType - without this included an exception is thrown

):
return type(self)(pa_type.storage_type).type
raise NotImplementedError(pa_type)

Expand Down
4 changes: 0 additions & 4 deletions pandas/core/internals/construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,10 +967,6 @@ def convert(arr):
# i.e. maybe_convert_objects didn't convert
convert_to_nullable_dtype = dtype_backend != "numpy"
arr = maybe_infer_to_datetimelike(arr, convert_to_nullable_dtype)
if convert_to_nullable_dtype and arr.dtype == np.dtype("O"):
new_dtype = StringDtype()
arr_cls = new_dtype.construct_array_type()
arr = arr_cls._from_sequence(arr, dtype=new_dtype)
elif dtype_backend != "numpy" and isinstance(arr, np.ndarray):
if arr.dtype.kind in "iufb":
arr = pd_array(arr, copy=False)
Expand Down
41 changes: 41 additions & 0 deletions pandas/tests/io/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -4358,3 +4358,44 @@ def test_xsqlite_if_exists(sqlite_buildin):
(5, "E"),
]
drop_table(table_name, sqlite_buildin)


@pytest.mark.parametrize("con", all_connectable)
def test_bytes_column(con, dtype_backend, request):
# GitHub Issue #59242
conn = request.getfixturevalue(con)
pa = pytest.importorskip("pyarrow")

val = b"\x01#Eg\x89\xab\xcd\xef\x01#Eg\x89\xab\xcd\xef"
Copy link
Member

Choose a reason for hiding this comment

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

What does #Eg represent?

Choose a reason for hiding this comment

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

this is the raw bytes returned from the hex string "0123456789abcdef0123456789abcdef" - I've changed the test to use bytes.fromhex("0123456789abcdef0123456789abcdef") to make things more clear here

if "postgres" in con:
if "adbc" in con:
val = b"\x00\x00\x00\x80\x01#Eg\x89\xab\xcd\xef\x01#Eg\x89\xab\xcd\xef"
Copy link
Member

Choose a reason for hiding this comment

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

Why does postgres have different content than the other databases?

Choose a reason for hiding this comment

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

the x'...' syntax in sqlite and mysql represents the same data (called a blob literal in sqlite ]) but in postgres this is slighly different in that it's not a bytea type , but instead a bit. Most of the drivers return this as a string representation of the bits (which is why val = "0000000100100011010001..." in one of the cases)

else:
val = (
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be the binary representation of the bytes? I'm a bit confused why this test has so many different expected values

Choose a reason for hiding this comment

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

yeah exactly and this is due to postgres returning the bit type which most of the drivers turn into this string

"0000000100100011010001010110011110001001101010"
"11110011011110111100000001001000110100010101100"
"11110001001101010111100110111101111"
)

if dtype_backend == "pyarrow":
dtype = pd.ArrowDtype(pa.binary())
if "postgres" in con:
if "adbc" in con:
dtype = pd.ArrowDtype(pa.opaque(pa.binary(), "bit", "PostgreSQL"))
else:
dtype = pd.ArrowDtype(pa.string())
else:
dtype = "O"
if "postgres" in con and "psycopg2" in con:
if dtype_backend == "numpy_nullable":
dtype = pd.StringDtype()
elif dtype_backend == lib.no_default and pd.options.future.infer_string:
dtype = pd.StringDtype(storage="pyarrow", na_value=np.nan)

expected = DataFrame([{"a": val}], dtype=dtype)
df = pd.read_sql(
"select x'0123456789abcdef0123456789abcdef' a",
conn,
dtype_backend=dtype_backend,
)
tm.assert_frame_equal(df, expected)
Loading