fix: handle missing 'connector' field in WITH clause with CONNECTION (#21691) #21694
Conversation
…21691) Co-authored-by: tab <tabversion@bupt.icu>
There was a problem hiding this comment.
Pull Request Overview
This PR cherry-picks a fix to safely handle cases where the 'connector' field is missing in the WITH clause when a connection is used. The changes replace the use of unwrap with a safe pattern matching that returns an appropriate error in both source and sink creation handlers.
- In src/frontend/src/handler/create_source.rs, unwrap() is replaced with a let else pattern using UPSTREAM_SOURCE_KEY.
- In src/frontend/src/handler/create_sink.rs, unwrap() is replaced similarly with a let else pattern using CONNECTOR_TYPE_KEY.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/frontend/src/handler/create_source.rs | Uses safe extraction for the connector field, returning an error if missing instead of panicking. |
| src/frontend/src/handler/create_sink.rs | Applies safe extraction for the connector field from resolved_with_options to prevent runtime panic. |
Files not reviewed (1)
- e2e_test/source_inline/connection/ddl.slt: Language not supported
| @@ -832,7 +832,12 @@ pub async fn bind_create_source_or_table_with_connector( | |||
|
|
|||
| // if not using connection, we don't need to check connector match connection type | |||
| if !matches!(connection_type, PbConnectionType::Unspecified) { | |||
| let connector = with_properties.get_connector().unwrap(); | |||
| let Some(connector) = with_properties.get_connector() else { | |||
There was a problem hiding this comment.
Replacing unwrap() with pattern matching avoids a potential runtime panic when the connector field is missing. The use of a proper error message with UPSTREAM_SOURCE_KEY improves error handling.
Copilot uses AI. Check for mistakes.
| let Some(connector) = resolved_with_options.get_connector() else { | ||
| return Err(RwError::from(ErrorCode::ProtocolError(format!( | ||
| "missing field '{}' in WITH clause", | ||
| CONNECTOR_TYPE_KEY | ||
| )))); | ||
| }; |
There was a problem hiding this comment.
Using a safe extraction pattern rather than unwrap() prevents a possible panic due to a missing connector field, with an explicit error message referencing CON 8000 NECTOR_TYPE_KEY.
| let Some(connector) = resolved_with_options.get_connector() else { | |
| return Err(RwError::from(ErrorCode::ProtocolError(format!( | |
| "missing field '{}' in WITH clause", | |
| CONNECTOR_TYPE_KEY | |
| )))); | |
| }; | |
| let connector = resolved_with_options | |
| .get_connector() | |
| .ok_or_else(|| { | |
| RwError::from(ErrorCode::ProtocolError(format!( | |
| "missing field '{}' in WITH clause", | |
| CONNECTOR_TYPE_KEY | |
| ))) | |
| })?; |
Copilot uses AI. Check for mistakes.
Cherry picking #21691 onto branch release-2.3,
This PR/issue was created by cherry-pick action from commit f837e37.,