Skip to content

Comments

Procmacro to derive unary/binary function definitions#32083

Merged
antiguru merged 2 commits intoMaterializeInc:mainfrom
antiguru:sqlfunc_derive
Apr 4, 2025
Merged

Procmacro to derive unary/binary function definitions#32083
antiguru merged 2 commits intoMaterializeInc:mainfrom
antiguru:sqlfunc_derive

Conversation

@antiguru
Copy link
Member

@antiguru antiguru commented Apr 3, 2025

Similar to the sqlfunc! macro, this proc macro derives the helper traits for unary and binary functions.

Here's a binary function for which we generate the EagerBinaryFunc trait implementation:

#[sqlfunc(is_monotone = (true, true), output_type = i16, is_infix_op = true, sqlname="+")]
fn add_int16<'a>(a: Datum<'a>, b: Datum<'a>) -> Result<Datum<'a>, EvalError> {
    a.unwrap_int16()
        .checked_add(b.unwrap_int16())
        .ok_or(EvalError::NumericFieldOverflow)
        .map(Datum::from)
}

The parameters control the behavior of the code that we generate and allow the function to override the default behavior.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@antiguru antiguru requested review from a team and ParkMyCar as code owners April 3, 2025 15:20
@antiguru antiguru requested a review from petrosagg April 3, 2025 15:21
@antiguru antiguru force-pushed the sqlfunc_derive branch 4 times, most recently from 9c0ed46 to 40bf133 Compare April 4, 2025 13:36
@antiguru antiguru requested a review from a team as a code owner April 4, 2025 13:36
@antiguru antiguru force-pushed the sqlfunc_derive branch 3 times, most recently from 09c97e5 to 8c18315 Compare April 4, 2025 14:18
Copy link
Contributor

@ParkMyCar ParkMyCar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging could you take a pass on the comments? Especially would love to see the documentation improved but overall love this PR!

antiguru added 2 commits April 4, 2025 21:22
Similar to the `sqlfunc!` macro, this proc macro derives the helper
traits for unary and binary functions.

Signed-off-by: Moritz Hoffmann <mh@materialize.com>
Signed-off-by: Moritz Hoffmann <mh@materialize.com>
@antiguru antiguru enabled auto-merge (squash) April 4, 2025 19:44
@antiguru antiguru merged commit 5f07a2a into MaterializeInc:main Apr 4, 2025
86 checks passed
@antiguru antiguru deleted the sqlfunc_derive branch April 7, 2025 07:41
def- added a commit that referenced this pull request Apr 7, 2025
Seen in
https://buildkite.com/materialize/nightly/builds/11711#0196084e-eaa8-4f23-8321-b063630a71ce

Follow-up to #32083

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered. ([trigger-ci for additional test/nightly
runs](https://trigger-ci.dev.materialize.com/))
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [ ] If this PR includes major [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note),
I have pinged the relevant PM to schedule a changelog post.

---------

Signed-off-by: Moritz Hoffmann <mh@materialize.com>
Co-authored-by: Moritz Hoffmann <mh@materialize.com>
antiguru added a commit that referenced this pull request Apr 11, 2025
Follow-up to #32083. Derive the SQL function glue code for more binary
functions.

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered. ([trigger-ci for additional test/nightly
runs](https://trigger-ci.dev.materialize.com/))
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [ ] If this PR includes major [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note),
I have pinged the relevant PM to schedule a changelog post.

---------

Signed-off-by: Moritz Hoffmann <mh@materialize.com>
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.

3 participants