Repository navigation
Conversation
antiguru
added this pull request to stack #38942
September 24, 2026 08:52
antiguru
force-pushed
the
sqlfunc-convert-range
branch
from
September 24, 2026 10:14
c9dca8f to
d0f0e6c
Compare
antiguru
force-pushed
the
sqlfunc-convert-map
branch
from
September 24, 2026 10:14
b75c5a0 to
a5afb0b
Compare
antiguru
force-pushed
the
sqlfunc-convert-map
branch
from
September 24, 2026 14:29
a5afb0b to
a669946
Compare
antiguru
force-pushed
the
sqlfunc-convert-range
branch
from
September 24, 2026 14:29
d0f0e6c to
7ed7d33
Compare
antiguru
force-pushed
the
sqlfunc-convert-map
branch
from
September 25, 2026 16:00
a669946 to
923e52e
Compare
antiguru
force-pushed
the
sqlfunc-convert-range
branch
from
September 25, 2026 16:00
7ed7d33 to
92629c6
Compare
antiguru
force-pushed
the
sqlfunc-convert-range
branch
from
October 14, 2026 09:11
92629c6 to
c7e3d11
Compare
antiguru
force-pushed
the
sqlfunc-convert-map
branch
from
October 14, 2026 09:11
923e52e to
b1f46ff
Compare
Both impls were hand-written because they allocate their result, which the eager trait now covers, and both checked NULL in the body, which under the macro has to come from the input type. `CastMapToString` takes `ExcludeNull<Datum<'a>>`, because `stringify_datum` takes a raw `Datum` and a bare `Datum` accepts NULL. `ExcludeNull` reports `nullable() == false`, so the derived `propagates_nulls` is true, matching the declaration, and its `try_from_result` turns `Datum::Null` into `Err(Ok(Datum::Null))`, which the evaluation layer returns as `Ok(Datum::Null)`. `introduces_nulls` derives false from the `String` inside the `Result`. Its `output_sql_type` returned `String.nullable(input_type.nullable)`, which is what the derived output plus the unary tail gives for a non-nullable input, so it needs no output modifier. `preserves_uniqueness = true` carries over, and `could_error` derives true from the fallible output, matching the `LazyUnaryFunc` default the impl inherited. `MapBuildFromRecordList` takes `DatumList<'a>`, which the body already required by unwrapping the input as a list. `DatumList` reports `nullable() == false` on its own, so no `ExcludeNull` wrapper is needed and the derived `propagates_nulls` is true. It returns `DatumMap<'a>`, because the body always builds a map. The impl declared `introduces_nulls` and `could_error` true, but the body returns NULL for no non-NULL input and cannot fail, so both now derive false from the `DatumMap` output. Its `output_sql_type` builds the map type from the payload, so it becomes an `output_type_expr`, which reports `nullable(false)` in place of the hardcoded `nullable(true)` and leaves nullability to the unary tail. Per-override accounting beyond the above, for both: `eval` becomes the function body minus the NULL check, and `preserves_uniqueness` (on `MapBuildFromRecordList`), `inverse`, `is_monotone` and `is_eliminable_cast` returned the trait defaults and are dropped. Both `Display` impls wrote a fixed string, so `sqlname` reproduces them, and `map_build` differs from the function name. With no `Display` left the file drops `std::fmt` along with the now unused `Eval`, `LazyUnaryFunc` and `SqlColumnType` imports. The registry reports `sqlfunc_signature` moving on both records. `map_build_from_record_list` also moves `introduces_nulls` and `could_error` to false and its output type to non-nullable, the corrections above, and the digest is regenerated in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
antiguru
force-pushed
the
sqlfunc-convert-map
branch
from
October 14, 2026 15:18
b1f46ff to
e61cc4e
Compare
antiguru
removed this pull request from stack #38942
October 14, 2026 15:45
Member
Author
|
Folded into #39062, which now carries all twenty file conversions as separate commits, per review feedback to land them as one PR. Posted by Claude Code. |
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
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.
Converts
CastMapToStringandMapBuildFromRecordListto#[sqlfunc]. Both were hand-writtenLazyUnaryFuncimpls because they allocate their result, which the arena passed toEagerUnaryFunc::callnow covers. Their NULL checks move into the input types,ExcludeNull<Datum<'a>>andDatumList<'a>.MapBuildFromRecordListreturnsDatumMap<'a>. It returns NULL only for a NULL input and cannot fail, but the old impl declaredintroduces_nullsandcould_errortrue and a nullable output. The registry records the correction: both flags become false and the output type is non-nullable. Otherwise onlysqlfunc_signaturemoves, on both records. The currentLIR_VERSIONhasn't shipped, so the snapshots are regenerated in place.Based on #39092.
🤖 Generated with Claude Code