8000 fix: print `AlterSinkOperation::SetSinkProps` correctly by tabVersion · Pull Request #21790 · risingwavelabs/risingwave · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: print AlterSinkOperation::SetSinkProps correctly #21790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 9, 2025

Conversation

tabVersion
Copy link
Contributor

…eses for changed properties

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

as title

introduced in #20691

What's changed and what's your intention?

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

@tabVersion tabVersion requested review from xxhZs and Copilot May 9, 2025 06:03
@github-actions github-actions bot added the type/fix Type: Bug fix. Only for pull requests. label May 9, 2025
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the output formatting of AlterSinkOperation::SetSinkProps so that sink properties are printed using a comma-separated format. It also updates the corresponding test data to reflect the new output string.

  • Replaced debug formatting with a call to display_comma_separated for sink properties.
  • Updated the testdata YAML file to match the new printed output format.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/sqlparser/tests/testdata/alter.yaml Updated formatted_sql to reflect the corrected output format for sink properties
src/sqlparser/src/ast/ddl.rs Changed the formatter from debug printing to using display_comma_separated for better output control
Comments suppressed due to low confidence (1)

src/sqlparser/src/ast/ddl.rs:481

  • Consider verifying that display_comma_separated handles edge cases such as an empty changed_props list to ensure consistent formatting.
write!(f, "CONNECTOR WITH ({})", display_comma_separated(changed_props))

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@tabVersion tabVersion enabled auto-merge May 9, 2025 06:05
@tabVersion tabVersion added this pull request to the merge queue May 9, 2025
Merged via the queue into main with commit 0b3ab2f May 9, 2025
28 of 29 checks passed
@tabVersion tabVersion deleted the tab/fix-print-setsinkprops branch May 9, 2025 07:48
github-actions bot pushed a commit that referenced this pull request May 22, 2025
Co-authored-by: tab <tabversion@bupt.icu>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

✅ Cherry-pick PRs (or issues if encountered conflicts) have been created successfully to all target branches.

github-merge-queue bot pushed a commit that referenced this pull request May 22, 2025
)

Co-authored-by: Bohan Zhang <tabvision@bupt.icu>
Co-authored-by: tab <tabversion@bupt.icu>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-cherry-pick-since-release-2.4 type/fix Type: Bug fix. Only for pull requests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0