feat: vector selector yacc rule #7

yuanbohan · 2022-12-11T14:47:45Z

What's Included

Sorry but this is a huge PR, the whole purpose of this PR is to show how to use grmtools and *.y file to generate PromQL parser.

This PR just focused on Parser part, and the Lexer is not implemented yet
Only NumberLiteral, StringLiteral, VectorSelector yacc rules are implemented
the promql.y is only part of promql's generated_parser.y so far, there is much work left to be done

Example

cargo run --example parser

// node_cpu_seconds_total{cpu=0,mode=idle}
AST: VectorSelector { name: Some("node_cpu_seconds_total"), offset: None, start_or_end: None, label_matchers: Matchers { matchers: [Matcher { op: Equal, name: "cpu", value: "0" }, Matcher { op: Equal, name: "mode", value: "idle" }, Matcher { op: Equal, name: "__name__", value: "node_cpu_seconds_total" }] } }

What's to do next

Lexer
the left parts of yacc generated Parser

waynexia

Splendid! I look roughly at the .y part, which generally looks good to me. And left a few comments in other parts.

src/parser/value.rs

src/parser/production.rs

src/util/duration.rs

src/parser/ast.rs

src/parser/value.rs

waynexia

LGTM 👍

waynexia · 2022-12-12T08:48:32Z

src/util/duration.rs

+            "1", "1y1m1d", "-1w", "1.5d", "d",
+            "",
+            // these are invalid in PromQL Go Version
+            // "294y", "200y10400w", "107675d", "2584200h",


We can un-comment this in another test, and assert they are valid in our version I suspose

waynexia · 2022-12-12T09:00:24Z

src/util/duration.rs

+            let v = cap.as_str().parse::<u64>().unwrap();
+            result = result + Duration::from_millis(v * millis);


If want to be corrected with any input, I suggest to use checked_add for result + Duration and checked_mul for v * millis. Because if it will overflow, it overflows here. So I'm afraid the check below (if result > Duration::MAX) is logically unreachable.

Yes, It's idiomatic Rust way

evenyag · 2022-12-12T09:07:42Z

src/label/matcher.rs

-            MatchType::MatchNotEqual => self.value.ne(s),
-            MatchType::MatchRegexp => todo!(),
-            MatchType::MatchNotRegexp => todo!(),
+pub fn new_matcher(token: Token, name: String, value: String) -> Result<Matcher, String> {


We may need to define an Error type for the parser instead of using String.

We can file a todo issue for this. I think we can leave them as are for now (i.e. keep using String). Parser error is a bit complex problem because it's part of an important user-facing UI, and we need to consider how to make it accurate and understandable.

evenyag · 2022-12-12T09:09:16Z

src/label/matcher.rs

+        T_EQL => Ok(Matcher::new(MatchOp::Equal, name, value)),
+        T_NEQ => Ok(Matcher::new(MatchOp::NotEqual, name, value)),
+        T_EQL_REGEX => {
+            let re = Regex::new(&value).map_err(|_| format!("illegal regex for {}", &value))?;


Also display the source error in the error message? It might contains some helpful message.

The error mechanism will be carefully re-designed in the future in Issue #9

yuanbohan · 2022-12-12T12:38:50Z

src/util/duration.rs

+    ]
+    .into_iter()
+    // map captured string to Option<Duration> iterator
+    // FIXME: None is ignored in closure. It is better to tell users which part is wrong.


I leave a FIXME here to resolve the issue in the future.

yuanbohan · 2022-12-12T12:56:31Z

I almost redo duration.rs. Any review comment is welcome.

killme2008

LGTM

yuanbohan added 7 commits December 10, 2022 00:21

example: simple parser with manual lexer

3dbc21d

yacc: production mod for functions in yacc action

3fec514

yacc: support duration production

128fe5b

doctest: for util::parse_duration

6d8e30d

yacc: number_literal expr

d7ef84a

yacc: format staff

a94d9a1

yacc: metric rules

c65d003

yuanbohan force-pushed the vector-selector-yacc-rule branch from 6c121dc to 56741ff Compare December 11, 2022 14:48

yuanbohan requested a review from waynexia December 11, 2022 14:48

yuanbohan force-pushed the vector-selector-yacc-rule branch from 56741ff to e2d9913 Compare December 11, 2022 15:05

yacc: vector_selector

e2d9913

waynexia reviewed Dec 12, 2022

View reviewed changes

src/parser/value.rs Outdated Show resolved Hide resolved

src/parser/production.rs Outdated Show resolved Hide resolved

src/util/duration.rs Outdated Show resolved Hide resolved

waynexia mentioned this pull request Dec 12, 2022

ci: license check in github action #8

Closed

misc: remove stale_nan, and exchange comment with doc

b890260

waynexia reviewed Dec 12, 2022

View reviewed changes

src/parser/ast.rs Outdated Show resolved Hide resolved

src/parser/ast.rs Outdated Show resolved Hide resolved

src/parser/value.rs Outdated Show resolved Hide resolved

yuanbohan force-pushed the vector-selector-yacc-rule branch 3 times, most recently from c51db71 to 5c66a88 Compare December 12, 2022 07:34

yuanbohan added 2 commits December 12, 2022 15:34

misc: omit the manual max_duration, use the rust Duration::MAX

5c66a88

misc: use crate instead of super

330d534

waynexia approved these changes Dec 12, 2022

View reviewed changes

waynexia reviewed Dec 12, 2022

View reviewed changes

evenyag reviewed Dec 12, 2022

View reviewed changes

yuanbohan commented Dec 12, 2022

View reviewed changes

yuanbohan force-pushed the vector-selector-yacc-rule branch 2 times, most recently from 8f1ac97 to 388d4b9 Compare December 12, 2022 12:48

yuanbohan force-pushed the vector-selector-yacc-rule branch from 388d4b9 to 13c1e37 Compare December 12, 2022 14:58

refactor: use iterator and chain to parse_duration

13c1e37

killme2008 approved these changes Dec 13, 2022

View reviewed changes

killme2008 merged commit 71d8a90 into develop Dec 13, 2022

killme2008 deleted the vector-selector-yacc-rule branch December 13, 2022 02:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: vector selector yacc rule #7

feat: vector selector yacc rule #7

yuanbohan commented Dec 11, 2022 •

edited

Loading

waynexia left a comment

waynexia left a comment

waynexia Dec 12, 2022

waynexia Dec 12, 2022

yuanbohan Dec 12, 2022

evenyag Dec 12, 2022

waynexia Dec 12, 2022 •

edited

Loading

evenyag Dec 12, 2022

yuanbohan Dec 12, 2022

yuanbohan Dec 12, 2022

yuanbohan commented Dec 12, 2022

killme2008 left a comment

		let v = cap.as_str().parse::<u64>().unwrap();
		result = result + Duration::from_millis(v * millis);

feat: vector selector yacc rule #7

feat: vector selector yacc rule #7

Conversation

yuanbohan commented Dec 11, 2022 • edited Loading

What's Included

Example

What's to do next

waynexia left a comment

Choose a reason for hiding this comment

waynexia left a comment

Choose a reason for hiding this comment

waynexia Dec 12, 2022

Choose a reason for hiding this comment

waynexia Dec 12, 2022

Choose a reason for hiding this comment

yuanbohan Dec 12, 2022

Choose a reason for hiding this comment

evenyag Dec 12, 2022

Choose a reason for hiding this comment

waynexia Dec 12, 2022 • edited Loading

Choose a reason for hiding this comment

evenyag Dec 12, 2022

Choose a reason for hiding this comment

yuanbohan Dec 12, 2022

Choose a reason for hiding this comment

yuanbohan Dec 12, 2022

Choose a reason for hiding this comment

yuanbohan commented Dec 12, 2022

killme2008 left a comment

Choose a reason for hiding this comment

yuanbohan commented Dec 11, 2022 •

edited

Loading

waynexia Dec 12, 2022 •

edited

Loading