Advanced SQL Analytics Framework #9
Conversation
… vulnerability (#94942) * disable sql expressions remove duckdb ref * Run `make update-workspace` --------- Co-authored-by: Scott Lepper <[email protected]>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| 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 enableSqlExpressions function contains a logic error that prevents SQL expressions from ever being enabled. The function always returns false regardless of the feature flag's value:
func enableSqlExpressions(h *ExpressionQueryReader) bool {
enabled := !h.features.IsEnabledGlobally(featuremgmt.FlagSqlExpressions)
if enabled {
return false
}
return false
}To correctly implement the feature flag check, consider simplifying to:
func enableSqlExpressions(h *ExpressionQueryReader) bool {
return h.features.IsEnabledGlobally(featuremgmt.FlagSqlExpressions)
}This will properly return true when the feature flag is enabled and false otherwise.
| func enableSqlExpressions(h *ExpressionQueryReader) bool { | |
| enabled := !h.features.IsEnabledGlobally(featuremgmt.FlagSqlExpressions) | |
| if enabled { | |
| return false | |
| } | |
| return false | |
| } | |
| func enableSqlExpressions(h *ExpressionQueryReader) bool { | |
| return h.features.IsEnabledGlobally(featuremgmt.FlagSqlExpressions) | |
| } | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
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! |
|
This pull request has been automatically closed because it has not had any further activity in the last 2 weeks. Thank you for your contributions! |
2 participants
Test 9