Implement ProjectNode for KQL project/TraceQL select operations#1
Open
balajikris wants to merge 5 commits intomasterfrom
Open
Implement ProjectNode for KQL project/TraceQL select operations#1balajikris wants to merge 5 commits intomasterfrom
balajikris wants to merge 5 commits intomasterfrom
Conversation
- Add ProjectNode and ProjectionExpression classes to CommonAST - Add ExpressionType enum for Expression Evaluation engine compatibility - Add AstBuilder factory methods for creating project operations - Update Graphviz generation to visualize ProjectNode structure - Add comprehensive examples for both KQL and TraceQL syntax - Implement Level 1 support: simple fields, aliases, basic arithmetic, simple functions - Type information only required for calculated fields (not simple field selections) - Add extensive test coverage for all ProjectNode functionality Key features: ✓ Engine-agnostic design compatible with Arrow data operations ✓ Grammar-driven implementation based on KQL/TraceQL analysis ✓ Support for field aliases and calculated expressions ✓ Type system for Expression Evaluation engine ✓ Cross-language compatibility (project/select keywords) ✓ Level 2 features documented as future TODOs Design Process Documentation: - Document collaborative design approach in memory-bank/designProcess.md - Update progress tracking in memory-bank/progress.md - Enhance Cline instructions with design collaboration guidance - Add ProjectNodeTests.cs with functional validation examples
- Remove ProjectNodeTests.cs from root directory - File is now properly located in tests/CommonAST.Tests/ProjectNodeTests.cs
balajikris
force-pushed
the
balajik/projections
branch
from
a3d87cc to
ae41089
Compare
balajikris
commented
Jul 8, 2025
Fixes and completions: 1. Remove incorrect ProjectNodeTests.cs (was console app, not tests) 2. Clean up duplicate ProjectNode tests in CommonASTTests.cs 3. Add complete KQL ProjectOperator support to KqlToCommonAstVisitor: - Handle project syntax: | project field1, alias = field2, calc = field3 / 1000 - Parse SimpleNamedExpression for aliases - Convert SeparatedElement<Expression> collections correctly - Generate proper ProjectNode and ProjectionExpression AST 4. Update memory bank documentation: - Mark KQL visitor support as complete in progress.md - Add comprehensive visitor pattern requirements in systemPatterns.md - Document critical visitor update requirements for future AST constructs Now supports end-to-end KQL project operation parsing: MyTable | project name, service_name = name, duration_ms = duration / 1000 Complete implementation chain: ✓ AST nodes (ProjectNode, ProjectionExpression) ✓ AstBuilder factory methods ✓ Examples (KQL and TraceQL) ✓ Comprehensive test coverage ✓ KQL visitor integration ✓ Memory bank documentation updates
- Added 'Project Node Tests' region with 10 core ProjectNode tests - Added 'Project Node Example Tests' region with 4 example validation tests - Tests cover all ProjectNode functionality: - Simple field projections - Aliases and calculated fields - ExpressionType coverage for all types - Mixed projection types (simple, alias, calculated, function calls) - Cross-language support (KQL project vs TraceQL select) - Empty projections edge case - All expression types in projections (Level 1 support) - Example validation for KQL and TraceQL scenarios Comprehensive test coverage ensures ProjectNode implementation is fully validated with proper MSTest framework tests (not console apps).
AmritHariharan
suggested changes
Jul 10, 2025
| /// <summary> | ||
| /// The expression to project (field name, calculation, function call, etc.) | ||
| /// </summary> | ||
| public required Expression Expression { get; set; } |
There was a problem hiding this comment.
Do we want to limit wildcard here or in the evaluation?
| /// <summary> | ||
| /// Keyword for language-specific syntax ('project' for KQL, 'select' for TraceQL) | ||
| /// </summary> | ||
| public string? Keyword { get; set; } |
There was a problem hiding this comment.
Didn't we ditch the keyword for some of the other operators as it was unnecessary?
src/AST/CommonAST.cs
Outdated
There was a problem hiding this comment.
Why not use LiteralKind there? All the return types should be (at least for now) the simple types we have defined. And once we add more complex types we want those to be supported everywhere as well.
Owner
Author
There was a problem hiding this comment.
I thought about this and felt its better to have a specific type for this. Reason being, the expression could be of a type dynamic or something else which may not have a literal value. I feel this could be possible, what do you think. While we may not be starting off with dynamic types, I think we have to design with extensibility in mind so that we don't have to introduce a breaking change in the future, as these are afterall public APIs.
Owner
Author
There was a problem hiding this comment.
so dynamic literals are valid, let me turn the question around. Null literal is valid.. but null is never a valid type for an expression.
| /// </summary> | ||
| public static ProjectionExpression CreateProjection(Expression expression, string? alias = null, ExpressionType? resultType = null) | ||
| { | ||
| return new ProjectionExpression |
There was a problem hiding this comment.
Add validation that expression isn't of type wildcard here.
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
2 participants
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.
Key features:
✓ Engine-agnostic design compatible with Arrow data operations
✓ Grammar-driven implementation based on KQL/TraceQL analysis
✓ Support for field aliases and calculated expressions
✓ Type system for Expression Evaluation engine
✓ Cross-language compatibility (project/select keywords)
✓ Level 2 features documented as future TODOs
Design Process Documentation: