Skip to content

fix(isthmus): keep the declared length of a wide character or binary type - #1169

Open
alexandrefimov wants to merge 3 commits into
substrait-io:mainfrom
alexandrefimov:issue-1167-varchar-precision
Open

fix(isthmus): keep the declared length of a wide character or binary type#1169
alexandrefimov wants to merge 3 commits into
substrait-io:mainfrom
alexandrefimov:issue-1167-varchar-precision

Conversation

@alexandrefimov

@alexandrefimov alexandrefimov commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

SubstraitTypeSystem overrides getMaxPrecision for the temporal types and DECIMAL; CHAR, VARCHAR and BINARY fell through to RelDataTypeSystemImpl, whose default caps them at 65536. The type factory then narrowed anything wider without raising an error, so a plan declaring varchar<2147483647> came back as VARCHAR(65536) and a consumer building a Calcite tree from it had no way to notice. The boundary is exact: 65536 round-trips, 65537 does not.

The cap reaches the conversion from SQL too, which the issue does not mention: Calcite narrows a declared width there the same silent way, so SELECT CAST(a AS VARCHAR(100000)) came out of the conversion as a varchar<65536>. It now comes out as varchar<100000>. What SQL is accepted does not change -- Calcite never rejected the wider declaration, it just did not keep it.

Those three are the types whose length crosses the boundary — holding fixedchar, varchar and fixed_binary — and Substrait declares each length as a 32-bit integer, so they now report Integer.MAX_VALUE. VARBINARY is left alone because Substrait's binary carries no length for it to lose. The issue was filed for the character types; fixed_binary turned out to have the same defect at the same boundary.

One consequence worth deciding on rather than discovering later. CHAR and BINARY are fixed-width, so Calcite pads a literal cast to them out to the declared length, and the padding is now allowed to reach the declared width. At the extreme, RelBuilder.project(cast('a' AS CHAR(2147483647))) throws OutOfMemoryError: Required length exceeds implementation limit, where the old cap silently made it CHAR(65536) and it converted. CHAR(100000) is fine, and the varying types are unaffected at any width. That is a genuine consequence of declaring a two-billion-character fixed-width value rather than something introduced here, but it does trade a silent wrong answer for a crash in that one case, and it is the ceiling question of #1124 with a number attached. If you would rather this landed as a narrower cap, or as a conversion that fails instead of narrowing, say which and I will redo it.

The conversion test asserts the resulting precision directly rather than through testType, whose expected type is built with the same type factory and would be narrowed alongside the value under test.

Closes #1167.

A RelDataTypeFactory narrows a width past its own maximum without saying so, and this conversion takes whatever factory it is handed, so raising the maximum fixes the narrowing only for the factory isthmus builds. A declared length the factory cannot hold is now reported instead of returned quietly.

The widths this admits are the spec's, not ones Calcite can always materialize: CHAR(2147483647) is a legal fixedchar and a legal Calcite type, but building a literal of it throws OutOfMemoryError inside RexBuilder.makeLiteral, which pads the value to the declared width. Before this, such a plan came back silently narrowed to 65536; now it either round-trips or fails where the width is materialized.

alexandrefimov and others added 3 commits August 28, 2026 10:37
…type

SubstraitTypeSystem overrides getMaxPrecision for the temporal types and
DECIMAL; CHAR, VARCHAR and BINARY fell through to RelDataTypeSystemImpl, whose
default caps them at 65536. The type factory then narrowed anything wider
without raising an error, so a plan declaring varchar<2147483647> came back as
VARCHAR(65536) and a consumer building a Calcite tree from it had no way to
notice. fixedchar and fixed_binary lose their length the same way.

Those three are the types whose length crosses the boundary, and Substrait
declares each as a 32-bit integer, so they now report Integer.MAX_VALUE.
VARBINARY is left alone: Substrait's binary carries no length for it to lose.

Closes substrait-io#1167.
The cap reaches the conversion from SQL too, which the tests did not say:
Calcite narrows a declared width to its maximum silently, so a cast wider
than the default used to leave the conversion as a varchar<65536>.
The conversion takes whatever RelDataTypeFactory it is handed, and a factory
caps a width at its type system's maximum without saying so, so raising the
maximum fixes the narrowing only for the factory isthmus builds. A fixedchar,
varchar or fixed_binary the factory narrows is now reported instead of returned.
@alexandrefimov
alexandrefimov force-pushed the issue-1167-varchar-precision branch from 895bc23 to cdca654 Compare August 28, 2026 08:27
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.

isthmus: a varchar or fixedchar wider than 65536 is silently narrowed converting to Calcite

1 participant