feat: jdbc sink user password #19822
Conversation
fuyufjh
added
user-facing-changes
This comment was marked as outdated.
This comment was marked as outdated.
| info.put("password", config.getPassword()); | ||
| } | ||
|
|
||
| try (Connection conn = DriverManager.getConnection(jdbcUrl, info); |
There was a problem hiding this comment.
I think you can directly use Connection getConnection(String url, String user, String password) and it has null check inside.
There was a problem hiding this comment.
Yeah, that's my original code (as written in the PR description). Here I use getConnection(jdbcUrl, info) is to unify the function call in this try (...), avoiding an ugly if...else.
There was a problem hiding this comment.
Don’t get it.😂 It seems Connection conn = DriverManager.getConnection(jdbcUrl, config.getUser(), config.getPassword()) is ok. correct me if I am wrong.
| info.put("password", config.getPassword()); | ||
| } | ||
|
|
||
| try (Connection conn = DriverManager.getConnection(jdbcUrl, info); |
There was a problem hiding this comment.
Don’t get it.😂 It seems Connection conn = DriverManager.getConnection(jdbcUrl, config.getUser(), config.getPassword()) is ok. correct me if I am wrong.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Fix #19766.
The root cause of #19766 is that JDBC URL is included in many JDBC Driver's error message, which is okay. But we put username and password in that URL, which is not a good practice.
This PR uses
getConnection(jdbcUrl, user, password)instead ofgetConnection(jdbcUrl), so that the error message won't show the password.No breaking change - the old syntax can work as well.
Checklist
Documentation
Need to update the docs of JDBC sink to use the new format.
No need to add release note - it's just a minor change.
Release note