feat(sql): migrate to DataFusion-based streaming SQL#219
Open
luoluoyuyu wants to merge 43 commits intoFunctionStream:mainfrom
Open
feat(sql): migrate to DataFusion-based streaming SQL#219luoluoyuyu wants to merge 43 commits intoFunctionStream:mainfrom
luoluoyuyu wants to merge 43 commits intoFunctionStream:mainfrom
Conversation
| StatementVisitorResult::Plan(Box::new(CreateTablePlan::new(plan))) | ||
| } | ||
| Err(e) => { | ||
| panic!("Failed to convert CREATE TABLE to logical plan: {e}"); |
There was a problem hiding this comment.
Planning errors cause server panic instead of error propagation
High Severity
Both visit_create_table and visit_streaming_table_statement call panic! when plan building fails. Since these errors can be triggered by invalid user-supplied SQL, any malformed CREATE TABLE or CREATE STREAMING TABLE statement will crash the entire server process. Every other error path in the visitor pattern properly propagates errors via Result or StatementVisitorResult.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Note
High Risk
Large dependency and planning/execution refactor: introduces DataFusion/Arrow/DataFusion-based streaming SQL planning plus persistent stream table catalog and job submission. Risk comes from new DDL paths (
CREATE STREAMING TABLE, connector-backedCREATE TABLE,DROP TABLE) and major crate/version bumps (Arrow 55/DataFusion git forks,bincodev2) affecting runtime behavior and serialization.Overview
Switches the SQL stack to a DataFusion-based streaming planner: adds compilation support for
CREATE STREAMING TABLE ... AS SELECT(including connector options likeconnector/partition_by) and connector-backedCREATE TABLE ... WITH ('connector'=...), plusDROP TABLEplanning.Wires these new plan nodes through coordinator execution by introducing a
CoordinatorRuntimeContext(task manager + stream catalog + job manager), persisting source/sink definitions to the stream catalog, and submitting streaming jobs when creating a streaming sink.Expands the
protocolcrate with new protobuf APIs (fs_api.proto,storage.proto) and build output (serde-derived types + descriptor set), adds a new Arrow-backedFsSchematype, and performs a major dependency refresh (Arrow 55, git-pinned DataFusion/Arrow/parquet/sqlparser/typify,bincode2 + new supporting crates) reflected inCargo.tomlandCargo.lock.Written by Cursor Bugbot for commit 7842995. This will update automatically on new commits. Configure here.