Advanced SQL Analytics Framework #2
Conversation
… vulnerability (#94942) * disable sql expressions remove duckdb ref * Run `make update-workspace` --------- Co-authored-by: Scott Lepper <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func enableSqlExpressions(h *ExpressionQueryReader) bool { | ||
| enabled := !h.features.IsEnabledGlobally(featuremgmt.FlagSqlExpressions) | ||
| if enabled { | ||
| return false | ||
| } | ||
| return false |
There was a problem hiding this comment.
The new SQL query branch now calls enableSqlExpressions, but that helper (lines 194‑199) always returns false regardless of whether featuremgmt.FlagSqlExpressions is enabled. As a result the reader immediately returns "sqlExpressions is not implemented" for every SQL query, so the SQL expression feature can no longer be used at all. The helper should return the flag state instead of unconditionally disabling the command.
Useful? React with 👍 / 👎.
| func (db *DB) TablesList(rawSQL string) ([]string, error) { | ||
| return nil, errors.New("not implemented") | ||
| } | ||
|
|
||
| func (db *DB) RunCommands(commands []string) (string, error) { | ||
| return "", errors.New("not implemented") | ||
| } | ||
|
|
||
| func (db *DB) QueryFramesInto(name string, query string, frames []*data.Frame, f *data.Frame) error { | ||
| return errors.New("not implemented") |
There was a problem hiding this comment.
The new pkg/expr/sql/db.go replaces the previous duckdb-backed implementation with stubs whose RunCommands, QueryFramesInto, and even TablesList methods simply return errors.New("not implemented"). Both sql.TablesList (used in NewSQLCommand) and SQLCommand.Execute now rely on these methods, so SQL expressions fail either at construction (TablesList errors) or when executing (QueryFramesInto errors). This removes the functionality entirely and needs a real implementation or the previous backend.
Useful? React with 👍 / 👎.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions! |
Test 9
Replicated from ai-code-review-evaluation/grafana-cursor#9