Skip to content

feat/pattern matching #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

feat/pattern matching #140

wants to merge 2 commits into from

Conversation

xav-db
Copy link
Member

@xav-db xav-db commented Jun 2, 2025

Description

Integrate Pattern matching into HQL that allow user to match on enum variants within queries. e.g.

// schema
N::User {}
N::Admin {}

Enum::UserType {
     Normal(User)
     Admin(Admin)
}

// query
QUERY patternMatch() => 
    N<User>::MATCH|_| {
        UserType::Normal(u) => ...,
        UserType::Admin(u) => ...,
    }
    RETURN NONE

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring
  • Other (please describe):

Related Issues

Closes #

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

@Copilot Copilot AI review requested due to automatic review settings June 2, 2025 06:12
@xav-db xav-db changed the title featPattern matching feat/pattern matching Jun 2, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates pattern matching and variable binding into HQL by extending the AST, parser, analyzer, and grammar.

  • Introduces VariableDeclaration, MatchStep, and related enums/structs in the parser AST.
  • Updates GraphStepType and StartNode to carry optional variable declarations.
  • Extends the grammar (grammar.pest) with enum_def and match_step constructs.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
helixdb/src/helixc/parser/helix_parser.rs Expanded StartNode and GraphStepType, added VariableDeclaration and pattern-matching logic.
helixdb/src/helixc/analyzer/analyzer.rs Updated analyzer pattern matches to use the new GraphStepType variants and variable metadata.
helixdb/src/grammar.pest Added enum_def, match_step, and variable_declaration rules to support pattern matching.
Comments suppressed due to low confidence (1)

helixdb/src/helixc/analyzer/analyzer.rs:2523

  • This error message appears in the InE branch but refers to an "outgoing" edge. Update it to mention "incoming" edge to accurately describe the scenario.
format!("Edge of type `{}` exists but it is not a valid outgoing edge type for node of type `{}`", edge_type, node_label),

Comment on lines +2479 to +2480
variable,
loc,
Copy link
Preview

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable (and similarly loc) binding in the OutE { edge_type, variable, loc } match arm is never used. Prefix unused bindings with an underscore to avoid compiler warnings (e.g., _variable).

Suggested change
variable,
loc,
_variable,
_loc,

Copilot uses AI. Check for mistakes.

Rule::variable_declaration => {
variable = Some(VariableDeclaration {
loc: p.loc(),
identifier: p.into_inner().next().unwrap().as_str().to_string(),
Copy link
Preview

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling .unwrap() after .ok_or_else(...) will panic on error; consider using the ? operator to propagate the ParserError instead of panicking.

Suggested change
identifier: p.into_inner().next().unwrap().as_str().to_string(),
identifier: p.into_inner().next()
.ok_or_else(|| ParserError::new("Expected identifier"))?
.as_str()
.to_string(),

Copilot uses AI. Check for mistakes.

@xav-db xav-db marked this pull request as draft June 2, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant