Skip to content

fix(jdbc): remove postgres jdbc sink schema if not specified - #20632

Merged
lmatz merged 3 commits into
mainfrom
dylan/remove_postgres_jdbc_sink_schema_if_not_specified
Mar 4, 2025
Merged

lmatz merged 3 commits into
mainfrom
dylan/remove_postgres_jdbc_sink_schema_if_not_specified

Conversation

@chenzl25

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?

  • Currently, we use public as the default schema for postgres jdbc sink sql. However, some of database like questdb doesn't support schema. I think we can just remove the schema from the dml if users haven't specified.

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.
  • My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

  • My PR needs documentation updates.
Release note

@chenzl25
chenzl25 requested a review from wenym1 February 27, 2025 06:25
@github-actions github-actions Bot added the type/fix Type: Bug fix. Only for pull requests. label Feb 27, 2025
@chenzl25
chenzl25 requested review from tabVersion and xxhZs February 27, 2025 06:25

@tabVersion tabVersion 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.

I think the fix may affect the existing jobs?
If a user rely on the behavior to connect to downstream, the fix can make the job fail after upgrade.

@chenzl25

Copy link
Copy Markdown
Contributor Author

I think the fix may affect the existing jobs? If a user rely on the behavior to connect to downstream, the fix can make the job fail after upgrade.

I think it is fine, because public should be the default schema.

@lmatz lmatz added the user-facing-changes Contains changes that are visible to users label Mar 3, 2025
@github-actions

github-actions Bot commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

Hi, there.

📝 Telemetry Reminder:
If you're implementing this feature, please consider adding telemetry metrics to track its usage. This helps us understand how the feature is being used and improve it further.
You can find the function report_event of telemetry reporting in the following files. Feel free to ask questions if you need any guidance!

  • src/frontend/src/telemetry.rs
  • src/meta/src/telemetry.rs
  • src/stream/src/telemetry.rs
  • src/storage/compactor/src/telemetry.rs
    Or calling report_event_common (src/common/telemetry_event/src/lib.rs) as if finding it hard to implement.
    ✨ Thank you for your contribution to RisingWave! ✨

This is an automated comment created by the peaceiris/actions-label-commenter. Responding to the bot or mentioning it won't have any effect.

@lmatz

lmatz commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

SCR-20250303-mb6

looks like questdb is not alone

let's label this as a breaking change

@lmatz
lmatz added this pull request to the merge queue Mar 3, 2025
@lmatz
lmatz removed this pull request from the merge queue due to a manual request Mar 3, 2025
@lmatz

lmatz commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

wonder if it is urgently required by a known customer?
If time allows, it is better to give a 1-version notice in advance before making a breaking change

@chenzl25

chenzl25 commented Mar 3, 2025 •

Copy link
Copy Markdown
Contributor Author

wonder if it is urgently required by a known customer? If time allows, it is better to give a 1-version notice in advance before making a breaking change

I think it is not that urgent.

@lmatz

lmatz commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

Got it,
as release-2.3 is cutoff, let's merge now and make it effective in v2.4

@lmatz
lmatz added this pull request to the merge queue Mar 4, 2025
Merged via the queue into main with commit 01d7008 Mar 4, 2025
@lmatz
lmatz deleted the dylan/remove_postgres_jdbc_sink_schema_if_not_specified branch March 4, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change type/fix Type: Bug fix. Only for pull requests. 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