feat: improve SQL validation for aggregate functions and entity model resilience#1576
Conversation
… resilience Add aggregate function detection (COUNT, SUM, AVG, MIN, MAX) to allow queries without WHERE/LIMIT clauses. Block COUNT(*) with a clear error message. Refactor projection parsing to use AST nodes instead of flattened tokens for more accurate column counting and wildcard detection. Make Entity model fields more resilient with optional SourceJoinCriteria fields, AliasChoices for sourceJoinCriterias, and union types for externalFields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b57049dc10
ℹ️ 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".
| if isinstance(node, (Identifier, IdentifierList)): | ||
| for child in node.tokens: | ||
| if isinstance(child, Function) and _is_count_star(child): | ||
| return True |
There was a problem hiding this comment.
Detect COUNT(*) inside aliased IdentifierList items
The COUNT(*) guard has the same traversal gap for IdentifierList: it checks direct Function children only and misses Identifier items that wrap aliased functions. As a result, forms like SELECT COUNT(*) AS total, COUNT(id) AS c FROM Customers LIMIT 10 can bypass the explicit COUNT(*) prohibition, even though this commit intends to block COUNT(*) universally.
Useful? React with 👍 / 👎.
…T(*) Aliased function expressions (e.g. COUNT(id) AS total) are parsed as Identifier > Function by sqlparse. The previous checks only looked at direct Function children, missing these cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c33c645 to
f08bed1
Compare
Summary
COUNT(*)with a clear error message directing users to useCOUNT(column_name)insteadFROMclause in all queriesEntitymodel more resilient: optionalSourceJoinCriteriafields,AliasChoicesforsourceJoinCriterias/sourceJoinCriteria, union types forexternalFields, and optionalreference_typeinFieldMetadatauipath-platformversion to 0.1.32Test plan
COUNT(*)rejection, missingFROM, and non-aggregate functions (UPPER, COALESCE) are validated🤖 Generated with Claude Code