diff --git a/src/include/duckdb_python/python_conversion.hpp b/src/include/duckdb_python/python_conversion.hpp index 962378e3..a1a0ff27 100644 --- a/src/include/duckdb_python/python_conversion.hpp +++ b/src/include/duckdb_python/python_conversion.hpp @@ -53,5 +53,10 @@ void TransformPythonObject(optional_ptr context, nb::handle ele, bool nan_as_null = true); Value TransformPythonValue(optional_ptr context, nb::handle ele, const LogicalType &target_type = LogicalType::UNKNOWN, bool nan_as_null = true); +string TransformPythonString(nb::handle str); +//! Needs no GIL +InvalidInputException UnencodableStrError(uint32_t codepoint, idx_t position); +//! Raises the pending UnicodeEncodeError of a failed UTF-8 conversion of str as the typed error above +[[noreturn]] void ThrowUnencodableStr(nb::handle str); } // namespace duckdb diff --git a/src/native/python_conversion.cpp b/src/native/python_conversion.cpp index d6883bbc..97ec82ab 100644 --- a/src/native/python_conversion.cpp +++ b/src/native/python_conversion.cpp @@ -56,6 +56,30 @@ static Value CastToTarget(Value val, const LogicalType &target_type) { target_type.ToString()); } +InvalidInputException UnencodableStrError(uint32_t codepoint, idx_t position) { + return InvalidInputException( + "Python Conversion Failure: cannot encode str as UTF-8, surrogate '\\u%04x' at position %d is not allowed", + codepoint, position); +} + +void ThrowUnencodableStr(nb::handle str) { + nb::python_error error; + Py_ssize_t start; + if (!error.matches(PyExc_UnicodeEncodeError) || PyUnicodeEncodeError_GetStart(error.value().ptr(), &start) != 0) { + throw error; + } + throw UnencodableStrError(PyUnicode_ReadChar(str.ptr(), start), NumericCast(start)); +} + +string TransformPythonString(nb::handle str) { + Py_ssize_t size; + const char *data = PyUnicode_AsUTF8AndSize(str.ptr(), &size); + if (data) { + return string(data, NumericCast(size)); + } + ThrowUnencodableStr(str); +} + static Value EmptyMapValue() { auto map_type = LogicalType::MAP(LogicalType::SQLNULL, LogicalType::SQLNULL); return Value::MAP(ListType::GetChildType(map_type), vector()); @@ -1071,7 +1095,7 @@ void TransformPythonObjectInternal(optional_ptr context, nb::hand break; } case PythonObjectType::String: { - auto stringified = nb::cast(ele); + auto stringified = TransformPythonString(ele); OP::HandleString(result, param, stringified); break; } diff --git a/src/numpy/numpy_bind.cpp b/src/numpy/numpy_bind.cpp index ebc8c890..955b4b79 100644 --- a/src/numpy/numpy_bind.cpp +++ b/src/numpy/numpy_bind.cpp @@ -51,7 +51,10 @@ void NumpyBind::Bind(ClientContext &context, nb::handle df, vector(nb::module_::import_("numpy").attr("unique")(column, false, true)); - vector enum_entries = nb::cast>(uniq.attr("__getitem__")(0)); + vector enum_entries; + for (auto entry : uniq.attr("__getitem__")(0)) { + enum_entries.push_back(TransformPythonString(nb::str(entry))); + } idx_t size = enum_entries.size(); Vector enum_entries_vec(LogicalType::VARCHAR, size); auto enum_entries_ptr = FlatVector::GetDataMutable(enum_entries_vec); diff --git a/src/numpy/numpy_scan.cpp b/src/numpy/numpy_scan.cpp index 352ca0a1..e8c88afc 100644 --- a/src/numpy/numpy_scan.cpp +++ b/src/numpy/numpy_scan.cpp @@ -391,8 +391,7 @@ void NumpyScan::Scan(ClientContext &context, PandasColumnBindData &bind_data, id // Zero copy: the bytes are the str's own or its cached UTF-8, both alive as long as the str auto utf8 = PyUnicode_AsUTF8AndSize(val, &size); if (!utf8) { - nb::python_error error; - throw InvalidInputException("Failed to convert a Python str to VARCHAR: %s", error.what()); + ThrowUnencodableStr(val_handle); } tgt_ptr[row] = string_t(utf8, UnsafeNumericCast(size)); } diff --git a/src/pandas/bind.cpp b/src/pandas/bind.cpp index f1062497..d06ce61b 100644 --- a/src/pandas/bind.cpp +++ b/src/pandas/bind.cpp @@ -3,6 +3,7 @@ #include "duckdb_python/pandas/column/pandas_numpy_column.hpp" #include "duckdb_python/numpy/numpy_array.hpp" #include "duckdb_python/pyconnection/pyconnection.hpp" +#include "duckdb_python/python_conversion.hpp" #include "duckdb_python/pyutil.hpp" namespace duckdb { @@ -75,7 +76,7 @@ static LogicalType BindColumn(ClientContext &context, PandasBindColumn &column_p // on the array throws). Iterating + nb::str handles both that and the legacy object[str] case. vector enum_entries; for (auto category : categories.GetArray()) { - enum_entries.push_back(nb::cast(nb::str(category))); + enum_entries.push_back(TransformPythonString(nb::str(category))); } idx_t size = enum_entries.size(); Vector enum_entries_vec(LogicalType::VARCHAR, size); diff --git a/src/pyconnection.cpp b/src/pyconnection.cpp index 3543d719..967963aa 100644 --- a/src/pyconnection.cpp +++ b/src/pyconnection.cpp @@ -1422,10 +1422,10 @@ std::unique_ptr DuckDBPyConnection::ReadCSV(const nb::object & if (has_sep && has_delimiter) { throw InvalidInputException("read_csv takes either 'delimiter' or 'sep', not both"); } - if (has_sep) { - bind_parameters["delim"] = Value(duckdb::PyUtil::CastToString(sep)); - } else if (has_delimiter) { - bind_parameters["delim"] = Value(duckdb::PyUtil::CastToString(delimiter)); + if (has_sep || has_delimiter) { + auto &delim = has_sep ? sep : delimiter; + bind_parameters["delim"] = + Value(nb::isinstance(delim) ? TransformPythonString(delim) : duckdb::PyUtil::CastToString(delim)); } if (!nb::none().is(files_to_sniff)) { @@ -1455,14 +1455,14 @@ std::unique_ptr DuckDBPyConnection::ReadCSV(const nb::object & if (!nb::isinstance(na_values) && !duckdb::PyUtil::IsListLike(na_values)) { throw InvalidInputException("read_csv only accepts 'na_values' as a string or a list of strings"); } else if (nb::isinstance(na_values)) { - null_values.push_back(Value(nb::cast(na_values))); + null_values.push_back(Value(TransformPythonString(na_values))); } else { nb::list null_list = nb::cast(na_values); for (auto elem : null_list) { if (!nb::isinstance(elem)) { throw InvalidInputException("read_csv 'na_values' list has to consist of only strings"); } - null_values.push_back(Value(nb::cast(nb::str(elem)))); + null_values.push_back(Value(TransformPythonString(nb::str(elem)))); } } bind_parameters["nullstr"] = Value::LIST(LogicalType::VARCHAR, std::move(null_values)); diff --git a/src/pyrelation.cpp b/src/pyrelation.cpp index c0aa6f52..02de2d93 100644 --- a/src/pyrelation.cpp +++ b/src/pyrelation.cpp @@ -4,6 +4,7 @@ #include "duckdb_python/pytype.hpp" #include "duckdb_python/pyresult.hpp" #include "duckdb_python/python_dependency.hpp" +#include "duckdb_python/python_conversion.hpp" #include "duckdb/parser/qualified_name.hpp" #include "duckdb/main/client_context.hpp" #include "duckdb/main/relation/query_relation.hpp" @@ -1235,7 +1236,7 @@ void DuckDBPyRelation::ToCSV(const string &filename, const nb::object &sep, cons if (!nb::isinstance(na_rep)) { throw InvalidInputException("to_csv only accepts 'na_rep' as a string"); } - options["null"] = {Value(nb::cast(na_rep))}; + options["null"] = {Value(TransformPythonString(na_rep))}; } if (!nb::none().is(header)) { diff --git a/tests/fast/pandas/test_pandas_object.py b/tests/fast/pandas/test_pandas_object.py index ffa63540..4f7179e9 100644 --- a/tests/fast/pandas/test_pandas_object.py +++ b/tests/fast/pandas/test_pandas_object.py @@ -30,7 +30,7 @@ def test_object_strings_of_every_utf8_width(self, duckdb_cursor): def test_object_string_with_a_lone_surrogate(self, duckdb_cursor): df = pd.DataFrame({"s": pd.Series(["fine", "x\ud800y"], dtype=object)}) # noqa: F841 - with pytest.raises(duckdb.InvalidInputException, match="surrogates not allowed"): + with pytest.raises(duckdb.InvalidInputException, match=r"surrogate '\\ud800' at position 1 is not allowed"): duckdb_cursor.sql("SELECT s FROM df").fetchall() assert duckdb_cursor.sql("SELECT 42").fetchall() == [(42,)] diff --git a/tests/fast/test_unencodable_strings.py b/tests/fast/test_unencodable_strings.py new file mode 100644 index 00000000..233f9335 --- /dev/null +++ b/tests/fast/test_unencodable_strings.py @@ -0,0 +1,236 @@ +"""A Python str that UTF-8 cannot encode is refused where it becomes a DuckDB string. + +A str can hold surrogate code points (U+D800 to U+DFFF), which have no UTF-8 encoding. Python keeps two surrogate +escapes written next to each other as two separate code points, so they are refused like a single one. +""" + +import re + +import pytest + +import duckdb +import numpy as np +import pandas as pd +from duckdb.sqltypes import BIGINT, VARCHAR + + +def execute_positional(con, text): + return con.execute("SELECT ?", [text]).fetchone()[0] + + +def executemany(con, text): + con.execute("CREATE TABLE t (i INTEGER, s VARCHAR)") + con.executemany("INSERT INTO t VALUES (?, ?)", [[1, "ok"], [2, text]]) + return con.execute("SELECT s FROM t WHERE i = 2").fetchone()[0] + + +def sql_params(con, text): + return con.sql("SELECT ?", params=[text]).fetchall()[0][0] + + +def execute_named(con, text): + return con.execute("SELECT $s", {"s": text}).fetchone()[0] + + +def value_parameter(con, text): + return con.execute("SELECT ?", [duckdb.Value(text, VARCHAR)]).fetchone()[0] + + +def list_element(con, text): + return con.execute("SELECT ?", [["a", text]]).fetchone()[0][1] + + +def dict_field(con, text): + return con.execute("SELECT ?", [{"f": text}]).fetchone()[0]["f"] + + +def struct_typed_value(con, text): + value = duckdb.Value({"f": text}, duckdb.struct_type({"f": VARCHAR})) + return con.execute("SELECT ?", [value]).fetchone()[0]["f"] + + +def map_value(con, text): + return con.execute("SELECT ?", [{"key": ["k"], "value": [text]}]).fetchone()[0]["k"] + + +def map_key(con, text): + (key,) = con.execute("SELECT ?", [{"key": [text], "value": [1]}]).fetchone()[0] + return key + + +def constant_expression(con, text): + return con.sql("SELECT 1").select(duckdb.ConstantExpression(text)).fetchall()[0][0] + + +def udf_result(con, text): + con.create_function("give", lambda _: text, [BIGINT], VARCHAR) + return con.sql("SELECT give(range) FROM range(1)").fetchall()[0][0] + + +def object_column(text): + return pd.DataFrame({"s": pd.Series(["a", text], dtype=object)}) + + +def pandas_register(con, text): + con.register("v", object_column(text)) + return con.sql("SELECT s FROM v").fetchall()[1][0] + + +def pandas_from_df(con, text): + return con.from_df(object_column(text)).fetchall()[1][0] + + +def pandas_object_list_column(con, text): + df = pd.DataFrame({"s": pd.Series([["a"], [text]], dtype=object)}) + return con.from_df(df).fetchall()[1][0][0] + + +def pandas_object_category(con, text): + categories = pd.Index(["a", text], dtype=object) + df = pd.DataFrame({"s": pd.Categorical.from_codes([0, 1], categories=categories)}) + return con.from_df(df).fetchall()[1][0] + + +def pandas_string_dtype_python(con, text): + df = pd.DataFrame({"s": pd.Series(["a", text], dtype=pd.StringDtype("python"))}) + return con.from_df(df).fetchall()[1][0] + + +def numpy_str_array(con, text): + arrays = {"s": np.array(["a", text])} # noqa: F841 + return con.sql("SELECT s FROM arrays").fetchall()[1][0] + + +def numpy_object_array(con, text): + arrays = {"s": np.array(["a", text], dtype=object)} # noqa: F841 + return con.sql("SELECT s FROM arrays").fetchall()[1][0] + + +CONVERSIONS = [ + execute_positional, + executemany, + sql_params, + execute_named, + value_parameter, + list_element, + dict_field, + struct_typed_value, + map_value, + map_key, + constant_expression, + udf_result, + pandas_register, + pandas_from_df, + pandas_object_list_column, + pandas_object_category, + pandas_string_dtype_python, + numpy_str_array, + numpy_object_array, +] + + +def read_csv_na_values(con, path, text): + con.read_csv(path, na_values=text) + + +def read_csv_na_values_list(con, path, text): + con.read_csv(path, na_values=["NA", text]) + + +def read_csv_delimiter(con, path, text): + con.read_csv(path, delimiter=text) + + +def write_csv_na_rep(con, path, text): + con.sql("SELECT NULL AS a").write_csv(path + ".out", na_rep=text) + + +CSV_OPTIONS = [read_csv_na_values, read_csv_na_values_list, read_csv_delimiter, write_csv_na_rep] + + +def refusal(escaped_char, position): + return re.escape(f"cannot encode str as UTF-8, surrogate '{escaped_char}' at position {position}") + + +@pytest.mark.parametrize("convert", CONVERSIONS) +def test_surrogate_is_refused(convert): + con = duckdb.connect() + with pytest.raises(duckdb.InvalidInputException, match=refusal("\\ud800", 0)): + convert(con, "\ud800") + assert con.execute("SELECT 42").fetchone() == (42,) + + +@pytest.mark.parametrize("option", CSV_OPTIONS) +def test_csv_option_with_surrogate_is_refused(option, tmp_path): + path = tmp_path / "in.csv" + path.write_text("a,b\n1,NA\n") + con = duckdb.connect() + with pytest.raises(duckdb.InvalidInputException, match=refusal("\\ud800", 0)): + option(con, str(path), "\ud800") + assert con.execute("SELECT 42").fetchone() == (42,) + + +@pytest.mark.parametrize( + ("text", "escaped_char", "position"), + [ + pytest.param("ab\udfffc", "\\udfff", 2, id="two_byte_kind"), + pytest.param("\U0001f600\udc00", "\\udc00", 1, id="four_byte_kind"), + pytest.param("\ud83d\ude00", "\\ud83d", 0, id="two_escapes"), + ], +) +@pytest.mark.parametrize("convert", [execute_positional, pandas_from_df]) +def test_refusal_names_the_first_surrogate(convert, text, escaped_char, position): + con = duckdb.connect() + with pytest.raises(duckdb.InvalidInputException, match=refusal(escaped_char, position)): + convert(con, text) + + +@pytest.mark.parametrize("text", ["a\x00b", "\U0001f600"], ids=["embedded_nul", "non_bmp"]) +@pytest.mark.parametrize("convert", CONVERSIONS) +def test_encodable_str_round_trips(convert, text): + assert convert(duckdb.connect(), text) == text + + +@pytest.mark.parametrize( + "text", ["\u00eb", "\u4e2d", "\U0001f600"], ids=["one_byte_kind", "two_byte_kind", "four_byte_kind"] +) +def test_object_column_of_each_str_kind_round_trips(text): + assert pandas_from_df(duckdb.connect(), text) == text + + +KIND_MARKERS = { + "one_byte_kind": ("\u00e9", 0xFF), + "two_byte_kind": ("\u0100", 0xFFFF), + "four_byte_kind": ("\U0001f600", 0x10FFFF), +} +UTF8_BOUNDARIES = [0x7F, 0x80, 0x7FF, 0x800, 0xD7FF, 0xE000, 0xFFFF, 0x10000, 0x10FFFF] + + +def scan_encoded(text): + return pandas_from_df(duckdb.connect(), text) + + +@pytest.mark.parametrize( + ("kind", "code_point"), + [ + pytest.param(kind, code_point, id=f"{kind}-U+{code_point:04X}") + for kind, (_, largest) in KIND_MARKERS.items() + for code_point in UTF8_BOUNDARIES + if code_point <= largest + ], +) +def test_object_column_round_trips_utf8_boundaries(kind, code_point): + text = KIND_MARKERS[kind][0] + chr(code_point) + assert scan_encoded(text) == text + + +@pytest.mark.parametrize("encoded_bytes", [12, 13], ids=["inline", "heap"]) +@pytest.mark.parametrize("kind", list(KIND_MARKERS)) +def test_object_column_round_trips_inline_and_heap_lengths(kind, encoded_bytes): + marker = KIND_MARKERS[kind][0] + text = marker + "a" * (encoded_bytes - len(marker.encode())) + assert scan_encoded(text) == text + + +def test_value_construction_does_not_check(): + assert duckdb.Value("\ud800", VARCHAR).object == "\ud800"