Skip to content

feat: support ||, position and overlay bytea functions - #22076

Merged
stdrc merged 1 commit into
risingwavelabs:mainfrom
guluguluhhhh:support-bytea-functions
Jun 11, 2025
Merged

stdrc merged 1 commit into
risingwavelabs:mainfrom
guluguluhhhh:support-bytea-functions

Conversation

@guluguluhhhh

@guluguluhhhh guluguluhhhh commented Jun 2, 2025 •

Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

add support for ||, position and overlay bytea functions in #8831

Checklist

  • I have written necessary rustdoc comments.
  • I have added necessary unit tests and integration tests.
  • I have added test labels as necessary.
  • I have added fuzzing tests or opened an issue to track them.
  • My PR contains breaking changes.
  • My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
  • I have checked the Release Timeline and Currently Supported Versions to determine which release branches I need to cherry-pick this PR into.

Documentation

  • My PR needs documentation updates.
Release note

support ||, position and overlay bytea functions

@guluguluhhhh
guluguluhhhh force-pushed the support-bytea-functions branch from 7800f5e to a03820c Compare June 2, 2025 11:40
@guluguluhhhh

Copy link
Copy Markdown
Contributor Author

@stdrc could you take a look when u r available😁

@stdrc
stdrc requested review from Copilot, stdrc and xiangjinwu June 8, 2025 13:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Add support for bytea-specific SQL functions: concatenation (||), position, and overlay.

  • Expose a new ExprType::ByteaConcatOp in the planner and binder
  • Implement bytea_concat_op, bytea_position, overlay_bytea, and overlay_for_bytea in the expression layer
  • Update expr.proto to include the new BYTEA_CONCAT_OP node

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/frontend/src/optimizer/plan_expr_visitor/strong.rs Recognize ExprType::ByteaConcatOp in the planner
src/frontend/src/expr/pure.rs Treat Type::ByteaConcatOp as a pure expression
src/frontend/src/binder/expr/binary_op.rs Bind `bytea
src/expr/impl/src/scalar/position.rs Add bytea_position function and basic tests
src/expr/impl/src/scalar/overlay.rs Add overlay_bytea, overlay_for_bytea and tests
src/expr/impl/src/scalar/concat_op.rs Add bytea_concat_op and tests
proto/expr.proto Define BYTEA_CONCAT_OP in the protobuf enum
Comments suppressed due to low confidence (2)

src/expr/impl/src/scalar/position.rs:79

  • Add a unit test for the case where sub_bytea is empty to verify the function returns 1 as documented.
pub fn bytea_position(bytea: &[u8], sub_bytea: &[u8]) -> i32 {

src/expr/impl/src/scalar/overlay.rs:198

  • Add a unit test that calls overlay_bytea(..., start = 0) or overlay_for_bytea(..., start = 0, ...) to confirm it returns the expected InvalidParam error.
if start <= 0 {

@stdrc stdrc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation LGTM! Thanks!

Comment thread src/expr/impl/src/scalar/overlay.rs
@guluguluhhhh
guluguluhhhh force-pushed the support-bytea-functions branch from a03820c to 1039726 Compare June 10, 2025 07:28
@stdrc
stdrc added this pull request to the merge queue Jun 10, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 10, 2025
@stdrc
stdrc added this pull request to the merge queue Jun 10, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 10, 2025
@stdrc
stdrc added this pull request to the merge queue Jun 11, 2025
Merged via the queue into risingwavelabs:main with commit 05e5121 Jun 11, 2025
@stdrc stdrc added the user-facing-changes Contains changes that are visible to users label Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

user-facing-changes Contains changes that are visible to users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants