Add parse_multipart_identifier function to parser

[Jefffrey]

Closes #805

To be used for parsing identifiers which when could be quoted, can get quite complex (rather than relying on simple split on dot)

Pulled from DataFusion with one change: allow whitespaces between parts in the identifier, as seems this is supported by Spark SQL and Postgres

So following idents would be parsed with the same result:

catalog.database.table

catalog . database . table

[coveralls]

Pull Request Test Coverage Report for Build 4969813312

• 48 of 54 (88.89%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.009%) to 86.181%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/parser.rs	48	54	88.89%

Totals
Change from base Build 4931911477:	0.009%
Covered Lines:	14344
Relevant Lines:	16644

---

💛 - Coveralls

[ankrgyl]

From what I can tell this can only be invoked directly (by calling parse_multipart_identifier()) and not automatically while parsing a query. Is that correct/intended?

[Jefffrey]

Yes that's correct @ankrgyl

It was partly inspired by how Spark has a similar method: https://github.com/apache/spark/blob/f8604ad14b24e8c657a0305b4fb8ad7efcb84060/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala#L65-L70

To allow downstream to parse identifiers by themselves (not necessarily as part of a query)

[ankrgyl]

Ah got it. Personally I'd prefer to integrate it into the main parser so that we increase our overall support for Spark SQL and Postgres. @alamb wdyt?

[alamb]

Ah got it. Personally I'd prefer to integrate it into the main parser so that we increase our overall support for Spark SQL and Postgres. @alamb wdyt?

Having it integrated makes sense to me in theory. Do you have any suggestion about how we might do this specifically @ankrgyl ?

[alamb]

Looking at this function more closely, I wonder how if we can't reuse parse_identifiers from above?

Or maybe we could add some more documentation (like docstrings / tests) for this function to make it clearer it is designed to parse strings into identifiers 🤔

[Jefffrey]

I think parse_identifiers above is the odd one out, as it accepts any delimiter between words/idents (except equal sign) which means it would parse test + one + two as a valid sequence of identifiers. This is why I introduced the new parse_multipart_identifier to be more strict about what constitutes a compound/multipart identifier.

Though I did base parse_multipart_identifier primarily on Spark SQL/Postgres syntax, so maybe parse_identifiers is generic to accommodate some other type of syntax

[alamb]

Upon some more reflection, I think we should accept this code and update the documentation strings on parse_multipart_identifier and parse_identifiers to explain in more detail what they do and how they are different.

I can try and find time to update the documentation maybe next week -- or @Jefffrey do you have time to do so?

[Jefffrey]

I've update the doc a bit, let me know any further suggestions

[alamb]

Upon some more reflection, I think we should accept this code and update the documentation strings on parse_multipart_identifier and parse_identifiers to explain in more detail what they do and how they are different.

I can try and find time to update the documentation maybe next week -- or @Jefffrey do you have time to do so?

[alamb]

I think this is a great addition -- thanks again @Jefffrey

I'll plan to merge (and release) this next week unless there are any more comments. cc @ankrgyl

[alamb]

This is great. Thank you