fix: keep the element type of a variadic tuple - #1963
Open
shcheklein wants to merge 4 commits into
Open
Conversation
tuple[T, ...] carries Ellipsis as its second argument to mark variable length,
but _list_to_array passed the whole argument list to list_of_args_to_type, which
tries to map each one to a SQL type. Ellipsis raises, the handler falls back to
JSON, and the declared element type is discarded:
list[int] Array(Int64)
tuple[int, ...] Array(JSON)
tuple[tuple[int, ...], ...] Array(JSON)
An Array(JSON) column stores each element as its own JSON document, so a tuple
of numbers was written as ["1","2"] where the same numbers in a list were
written [1,2], and a filter comparing against the natural value found nothing.
Ellipsis is dropped before the element type is inferred, so a variadic tuple
resolves to what its list counterpart resolves to. Fixed-length tuples are
unaffected: they have no Ellipsis, and a mixed one still falls back to JSON.
Datasets written before this keep their stored schema and read back unchanged.
Deploying datachain with
|
| Latest commit: |
bc9fcca
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b8497cf8.datachain-2g6.pages.dev |
| Branch Preview URL: | https://fix-variadic-tuple-element-t.datachain-2g6.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Aug 31, 2026
shcheklein
added a commit
that referenced
this pull request
Sep 1, 2026
list[Annotated[str, "meta"] | Literal[None]] raised "Cannot recognize type" before nullability was ever considered: the item type was resolved from the annotation as written, and neither the scalar table nor the union handling knows that composition, though each half is recognized alone. Pydantic accepts the annotation and ["x", None] with it. Peel first, then resolve. Ellipsis is deliberately left in place -- it is what marks a variadic tuple, and dropping it here would quietly turn tuple[int, ...] into Array(Int64), which is #1963's change to make with its own migration. This raised no TypeError on main either, so it is a gap rather than a regression. Declared types are unchanged across seventeen annotations and storage across fourteen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stripping Ellipsis leaves tuple[()] with no argument to read, which is refused rather than indexed into. The branch had no test, which is what codecov was reporting; main reaches the same annotation as IndexError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tuple[int, ...]is stored asArray(JSON)rather thanArray(Int64)....is a real object —Ellipsis— so it turns up as an actual entry in thetype's arguments:
_list_to_arrayiterated those looking for a single element type, sawintandEllipsis, and concluded "mixed", which is the correct answer fortuple[int, str]and the wrong one here. So the annotation that means the sameshape as
list[int]was the one that did not behave like it.Dropping
Ellipsisbefore the element type is read:tuple[int, ...]Array(JSON)Array(Int64)tuple[str, ...]Array(JSON)Array(String)tuple[int | None, ...]Array(JSON)Array(Nullable(Int64))tuple[int, str]Array(JSON)list[int]Array(Int64)tuple[()]names no element type at all and is refused rather than indexedinto;
mainreaches it asIndexError.Compatibility
This changes the physical type of an existing annotation, so it is a storage
boundary, not only an improvement:
Array(JSON)holds["1","2"]; the read path accepts that and hydrates(1, 2)as before.and one written after hold
["1","2"]and[1,2]for the same value. Theyhydrate identically, and both declare the same schema, but
merge(on=...),distinct,subtractand grouping compare the stored text — so they disagree.Nothing marks which codec a column was written with.
old.mutate(copy=C("r.scores")).to_list("copy")gives('1', '2')where adirect read gives
(1, 2).This is the premise for the array-codec work in
#1968: while a variadic
scalar tuple is accidentally
Array(JSON), the JSON element rule cannot be madeuniform without that accident driving it.