Support Snowflake MATCH_RECOGNIZE syntax

[jmhain]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 8773731659

Details

• 354 of 391 (90.54%) changed or added relevant lines in 7 files are covered.
• 651 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+1.1%) to 89.202%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/dialect/generic.rs	1	2	50.0%
src/dialect/mod.rs	1	2	50.0%
src/dialect/snowflake.rs	1	2	50.0%
src/ast/query.rs	64	74	86.49%
tests/sqlparser_common.rs	136	146	93.15%
src/parser/mod.rs	143	157	91.08%

Files with Coverage Reduction	New Missed Lines	%
src/parser/mod.rs	147	92.66%
tests/sqlparser_common.rs	504	89.59%

Totals
Change from base Build 8772950293:	1.1%
Covered Lines:	23163
Relevant Lines:	25967

---

💛 - Coveralls

[iffyio]

thanks @jmhain! I think this looks good, left a couple minor comments/questions

[iffyio]

should this use the dialect_of check instead or is the trait method required for this feature?

[jmhain]

I prefer to use the trait method here because it can be reused in the test with all_dialects_where(|d| d.supports_match_recognize(). This way if it's added for another dialect in the future (e.g. Trino), those tests will run without having to remember to add the new dialect to them.

[iffyio]

Ah I see, not sure if we have a guideline for this but feels to me like the dialect trait could get bloated quickly otherwise. Not sure though, deferring to @alamb if you have thoughts (and the PR looks good to me for a review too!)

[jmhain]

Yeah that's a good point. We could do something like this:

struct FeatureSupport {
    match_recognize: bool,
    // other features
}

trait Dialect {
    fn features() -> &'static FeatureSupport;

    // methods requiring custom code
}

If FeatureSupport gets too big, we can make it hierarchical.

[alamb]

I don't have a great story for this question -- it seems to me that the crate is currently inconsistent where sometimes it uses a trait method and sometimes uses the hard coded IsDialect ..

I think the FeatureSupport idea is neat, and if we didn't already have 2 patterns it would probably be the way to go. However, given we already have 2 methods, I think adding a third would be less than ideal

Ideally we would have a single way and I think the best thing for users of the crate is methods on Dialect (mostly so they can define their own custom dialects, which I have seen some of them do). However, I fully acknowledge that the trait will get very large for sure.

However, a wholesale refactoring of the crate is probably not feasible at this time.

If we can get some consensus between the three of us maybe I can add something to the documentation on the preferred method (dialectof or trait method)

[iffyio]

The FeatureSupport proposal seems like it balances the tradeoffs closely but yeah, agree that its likely not ideal to introduce another mechanism for this. At least the Dialect trait methods route sounds very reasonable to me as a preferred method in that case since as mentioned it helps users with custom dialect setup and it makes it easier to add test coverage for dialects.

[alamb]

Thanks @jmhain and @iffyio

[alamb]

I don't have a great story for this question -- it seems to me that the crate is currently inconsistent where sometimes it uses a trait method and sometimes uses the hard coded IsDialect ..

I think the FeatureSupport idea is neat, and if we didn't already have 2 patterns it would probably be the way to go. However, given we already have 2 methods, I think adding a third would be less than ideal

Ideally we would have a single way and I think the best thing for users of the crate is methods on Dialect (mostly so they can define their own custom dialects, which I have seen some of them do). However, I fully acknowledge that the trait will get very large for sure.

However, a wholesale refactoring of the crate is probably not feasible at this time.

If we can get some consensus between the three of us maybe I can add something to the documentation on the preferred method (dialectof or trait method)

[alamb]

🤯

[alamb]

@jmhain -- this PR looks like it has some conflicts -- would it be possible to resolve them?

[jmhain]

@alamb Yep! Done

[alamb]

Thanks @jmhain