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
[Feature] support create or replace pipe #37658
Conversation
Signed-off-by: Murphy <mofei@starrocks.com>
removePipe(pipe); | ||
repo.deletePipe(pipe); | ||
} | ||
|
||
public void dropPipesOfDb(String dbName, long dbId) { | ||
try { | ||
lock.writeLock().lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Attempting to replace a Pipe
may fail if pipeMap.get(nameToId.get(dbIdAndName))
returns null, leading to a NullPointerException.
You can modify the code like this:
@@ -74,8 +74,13 @@
} else if (stmt.isReplace()) {
LOG.info("Pipe {} already exist, replace it with a new one", stmt.getPipeName());
Long pipeId = nameToId.get(dbIdAndName.getFirst());
- Pipe pipe = pipeMap.get(pipeId);
+ Pipe pipe = pipeMap.get(dbIdAndName.getFirst());
+ if (pipe == null) {
+ throw new DdlException("Cannot replace non-existing pipe with name: " + dbIdAndName.getSecond());
+ }
dropPipeImpl(pipe);
+ }
}
// Add pipe
In the modification above, I've added a null check for pipe
before attempting to replace it. If pipe
is null, an exception is thrown indicating that the pipe cannot be replaced because it doesn't exist. This avoids the potential NullPointerException
. Additionally, make sure to use the correct key when obtaining pipe
from pipeMap
; the key should be derived from the resolved dbIdAndName
, specifically the first element (dbId
) in case dbIdAndName
is a pair of database ID and name as the context suggests.
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 4 New issues |
[FE Incremental Coverage Report]✅ pass : 15 / 15 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
@Mergifyio backport branch-3.2 |
✅ Backports have been created
|
Signed-off-by: Murphy <mofei@starrocks.com> (cherry picked from commit 3db4497)
Signed-off-by: Murphy <mofei@starrocks.com> Signed-off-by: 张敢 <zhanggan@deepexi.com>
Why I'm doing:
What I'm doing:
CREATE OR REPLACE PIPE
Fixes #37509
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: