Conversation
There was a problem hiding this comment.
Pull request overview
Updates the GitHub datasource schema for the pull-requests table to avoid using the state column’s operators for pushdown filtering, since PR state filtering via the existing operator/pushdown path is not behaving as expected.
Changes:
- Removed explicit
Operators: equalityOperatorsfrompull-requests.stateschema column so filtering can be handled in the SQL engine instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {Name: "deletions", Type: schemas.ColumnTypeInt64}, | ||
| {Name: "repository", Type: schemas.ColumnTypeString}, | ||
| {Name: "state", Type: schemas.ColumnTypeEnum, Values: []string{"OPEN", "CLOSED", "MERGED"}, Operators: equalityOperators}, | ||
| {Name: "state", Type: schemas.ColumnTypeEnum, Values: []string{"OPEN", "CLOSED", "MERGED"}}, |
There was a problem hiding this comment.
Removing Operators: equalityOperators here likely disables filter pushdown for PR state (so filtering happens in the SQL engine instead). That can significantly increase GitHub search result sizes/API usage (and rate-limit risk) for common queries; consider instead mapping the enum values to the correct GitHub search qualifiers (e.g., is:open, is:closed, is:merged) so state filtering can still be pushed down efficiently. If pushdown is intentionally disabled, add a short inline comment explaining why this column intentionally omits Operators to prevent someone from reintroducing the broken behavior later.
| {Name: "state", Type: schemas.ColumnTypeEnum, Values: []string{"OPEN", "CLOSED", "MERGED"}}, | |
| {Name: "state", Type: schemas.ColumnTypeEnum, Values: []string{"OPEN", "CLOSED", "MERGED"}, Operators: equalityOperators}, |
There was a problem hiding this comment.
I would do this but passing is:closed to GitHub also returns merged PRs 😞
The state operator for pull requests doesn't work as we expect so we will filter in the SQL engine instead.