Skip to content

Fix PySpark struct() to accept a list of columns - #378

Closed
wavebyrd wants to merge 612 commits into
duckdb:mainfrom
wavebyrd:fix/17189-spark-struct-list
Closed

wavebyrd wants to merge 612 commits into
duckdb:mainfrom
wavebyrd:fix/17189-spark-struct-list

Conversation

@wavebyrd

Copy link
Copy Markdown
Contributor

Fixes duckdb/duckdb#17189

The PySpark API's struct() function accepts either varargs or a single list of columns. The DuckDB implementation only accepted varargs, causing an InvalidInputException when a list was passed.

This applies the same unwrapping pattern already used by array() in this file: if a single list or set argument is passed, unwrap it before processing.

Changes

  • duckdb/experimental/spark/sql/functions.py: Added list/set unwrapping to struct() and updated type hints to match array()
  • tests/fast/spark/test_spark_column.py: Added test_struct_column_with_list covering the reported use case

Test plan

  • New test passes struct([df.age, df.name]) (list syntax) and verifies the output matches the expected struct rows
  • Existing test_struct_column continues to verify varargs usage

evertlammerts and others added 30 commits September 18, 2025 19:02
…ckdb#55)

Redo of duckdb#54 but now actually
pulling from my feature branch, not main, as I mistakenly did.

Includes the fix for the changes requested at
duckdb#54 (comment)
In this PR:
* Using an extensive set of rules (see pyproject.toml), fixed lots and
lots of linting errors with ruff
* Added pre-commit as a dev dependency, with support for ruff,
clang-format and cmake-format
* Added a linting step to the on-pr job
* Added the post-checkout hook as well that automatically updates the
git submodule.

Soon:
* add dev docs
* plug into on-pr workflow
* plug into nightly release workflow as sanity check (because why not:
it's fast)

This is step 1 in fixing some of the devexp issues we have since v1.4.0.

See duckdb#62
duckdb#57 and
duckdb#47
Fixes duckdb#74 

This PR:
- exports the `adbc_driver_duckdb` package so you can use it after `pip
install duckdb`
- doesn't just skip the tests if the package can't be found
- limits the exported symbols to the only two that are ever needed:
`duckdb_adbc_init` and `PyInit__duckdb`:

❯ nm -g -C -P
.venv/lib/python3.11/site-packages/_duckdb.cpython-311-darwin.so | ag -v
' U '
        _PyInit__duckdb T 41900 0
        _duckdb_adbc_init T 1054e6c 0
evertlammerts and others added 25 commits January 5, 2026 12:41
Fixes ADBC tests to align with changes made in
duckdb/duckdb#20344
Fixes duckdb/duckdb#20329

`DuckDBPyRelation.select_dtypes` failed for relations with column names
that
require quoting (e.g., names containing spaces). The projection builder
did
not correctly quote identifiers, leading to binder errors.

This change ensures identifiers are quoted consistently and adds a
regression
test to cover the reported case.
This fixes a bug in the cmake function that checks whether we should
include jemalloc in our extension list. Should make sure that jemalloc
not included on Windows.
The jemalloc extension's symbols can't be found. There might be a deeper
issue at play here, but there isn't really any reason to use jemalloc by
default, so let's remove that first.
This change makes all CMake targets to use `-MT`/`-MTd` compilation
flags for Windows MSVC builds. This way the MSVC runtime library is
linked statically and the workaround for VS2019 described in
duckdb/duckdb#17991 is no longer necessary.

`extension-ci-tools` PR: duckdb/extension-ci-tools#276

Ref: duckdblabs/duckdb-internal#2036
Fixes duckdb#209

The type stub for `DuckDBPyRelation.aggregate` incorrectly restricts the
`aggr_expr` parameter to `Expression | str`. However, the DuckDB Python
API
and runtime behavior also support passing a list of `Expression` objects
for
multiple aggregations.

The fix extends the type annotation to include `list[Expression]`,
aligning
the stub definition with the actual supported API behavior.
``` 
   def aggregate(
        self, aggr_expr: Expression | str | list[Expression], group_expr: Expression | str = ""
    ) -> DuckDBPyRelation: ...
```
Fixes duckdb#224 
Fixes the memory leak reported in
[https://github.com/duckdb/duckdb-python/issues/224](https://github.com/duckdb/duckdb-python/issues/224)

The issue was caused by Python UDF return values not being released
after conversion. While this affected all return types, it was only
observable for large VARCHAR/bytes values due to their size. The fix
ensures correct reference management using
`py::reinterpret_steal<py::object>.`

A regression test was added to detect refcount leaks in Python scalar
UDF execution.
The struct() function now unwraps a single list or set argument,
matching the PySpark API behavior and the existing array() function
in this codebase.
@wavebyrd
wavebyrd force-pushed the fix/17189-spark-struct-list branch from bf2c5a5 to 1f211e2 Compare March 13, 2026 21:22
evertlammerts added a commit to evertlammerts/duckdb-python that referenced this pull request Oct 17, 2026
…2.0 and 3.5.9 (duckdb#686)

The experimental Spark API (`duckdb.experimental.spark`) had drifted a
long way from what it claims to be, a drop-in subset of PySpark on
DuckDB. A clean install of 1.5.6 could not even import it: `_globals.py`
and `_typing.py` imported `typing_extensions` and `numpy` at module
level, neither of which the `duckdb` package declares (duckdb#653). Beyond
that, I graded the module against real Spark 4.2.0 on 64 typical
snippets and 47 edge cases, and found that 18 of the snippets raised and
12 of the edge cases returned a different value than Spark without any
error. Joins with repeated column names were the worst: after a second
join, or a `withColumn` or `drop`, both columns of a repeated name
silently carried the values of the first one.

This PR makes the module do what it says, with a fixed compatibility
target: PySpark 4.2.0 and 3.5.9. Where the two versions differ, the
module follows 4.2.0 unless it cannot. The rule for everything else is
simple. Behavior is mapped mechanically onto DuckDB where that gives
Spark's answer; where it does not, the module raises rather than
returning a different value; a small number of cases where DuckDB's
answer is deliberately kept are documented (38 of them, which I will put
on the Spark API docs page). The module does not re-implement Spark
semantics: an earlier version of this work had a CSV reader that parsed
values the way Spark does, and I took it out again in favor of DuckDB's
casts and type detection.

What changed, by area:

- Imports: no module-level dependency on anything the `duckdb` package
does not declare. pandas, pyarrow and pytz stay optional at the call
sites that need them. A test imports the module with each optional
package blocked.
- Names: every function and expression gets Spark's column name, and a
frame keeps Spark's names separate from DuckDB's, so repeated names
(`id, id`) survive instead of becoming `id, id_1`. Columns of a join
input are referenced by position, and every name resolution checks for
ambiguity and raises `AMBIGUOUS_REFERENCE` where Spark does.
- Values: integer casts truncate toward zero like Spark; set operations
match struct fields by position; timestamps are collected as epoch
microseconds and converted with Spark's own rules, so `collect()`,
`toPandas()` and `show()` agree with Spark in any process or session
time zone and no longer need pytz; dozens of function-level differences
in NULL handling, `sequence`, `regexp_replace`, padding, binary
`concat`, and so on.
- UDFs: `register` returns a usable callable, a UDF named like a builtin
neither shadows nor is replaced by it, struct arguments arrive as
`Row`s.
- Readers and writers: Spark's save modes (a plain `write.csv()` no
longer overwrites silently), `load()` by file extension and with a
schema (it returned all-NULL rows for a CSV with a schema), `_c0` column
names, `nullValue`, header checks against the schema, and option flags
that are either honored or raise instead of being accepted and ignored.
- Input: `createDataFrame` verifies rows and pandas input against the
schema the way Spark does; a list of dicts works (duckdb#183); a statement
without a result in `spark.sql()` gives a frame with an empty schema
(duckdb#428; the rest of that issue needs relations without columns in the
engine).
- Errors: Spark's error classes for the cases above, with seven classes
added to the module's table from Spark 4.2.0's definitions.

Four binding bugs came out of this work and are fixed in the same
change, all confirmed new:

- `StarExpression(exclude=[ColumnExpression("b", "id")])` dropped the
qualifier and excluded every `id`; the name parts now map to column,
table, schema and catalog, and an empty name or more than four parts
raise `ValueError`.
- `ColumnExpression("s.a.b.c")` with a single dotted string kept only
the first, the second to last and the last part on this branch (1.5.6
raised instead), so it bound another struct field. It now keeps every
part, as the multi-argument form does.
- `duckdb.struct_type({})` refused the empty struct, a check the engine
dropped long ago.
- A NaN inside a list, struct, map or typed `Value` parameter became
NULL, because the nested conversions dropped the `nan_as_null` flag the
parameter path sets.

Four open community PRs addressed parts of this, and I reimplemented
their fixes rather than merging them, because the surrounding code
changed under them: duckdb#552 (`show()` signature and the UDF return type
bug), duckdb#388 (inference for `createDataFrame` from dicts), duckdb#378
(`struct()` of a list) and duckdb#626 (path quoting in `load()`). The authors
are credited: wavebyrd's and Raaif-Yousuf's commits are cherry-picked,
and all four are co-authors of the main commit.

What this does not do: the module's coverage of PySpark is unchanged,
about a third of the API, with no `Window`; missing API raises
`AttributeError` or `ContributionsAcceptedError` as before. The engine's
relational API has a bug that this module contains with an extra
projection rather than fixes: an aggregate, filter or projection over a
`limit`, `order` or `distinct` of a join relation merges into the join's
own SELECT beneath the modifier, so
`join.limit(1).aggregate("count(*)")` gives the unlimited count. I will
file that separately.

Evidence. The Spark tests now run in three modes: against the module,
against real PySpark 4.2.0 and against real PySpark 3.5.9
(`USE_ACTUAL_SPARK=true`), plus a run in a non-UTC time zone. On this
tree: 1813, 1757 and 1745 tests pass, 0 fail (before: 289 tests, with 11
and 15 failures on the two Spark versions). PySpark's own function
doctests, run against the module with the names aliased, pass fully for
111 of 185 functions (3 before). The 64 snippets graded against Spark
4.2.0: 48 match, 16 raise, 0 differ silently (45, 18 and silent
differences before). The full `tests/fast` suite passes (8089 tests).
The binding fixes were checked under AddressSanitizer and LeakSanitizer
against an unmodified build, with no new report.

Resolves duckdb#653 and duckdb#183, partly duckdb#428.
@evertlammerts

Copy link
Copy Markdown
Member

Thanks @wavebyrd, overdue. Your commit has landed (in your name) as part of #686.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python] PySpark-API: struct(List[Column]) not working