Skip to content
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

How to best add support for IDENTIFIER() clause #1412

Open
kacpermuda opened this issue Sep 5, 2024 · 2 comments
Open

How to best add support for IDENTIFIER() clause #1412

kacpermuda opened this issue Sep 5, 2024 · 2 comments

Comments

@kacpermuda
Copy link
Contributor

kacpermuda commented Sep 5, 2024

I am thinking about adding support for the IDENTIFIER() clause (available in Snowflake and Databricks, see docs below).

In OpenLineage, I was able to partially support it by implementing visitors, but that only worked for a couple of statement types (SELECT, MERGE, UPDATE, DELETE) - the ones where i was able to access a TableFactor and not only ObjectName.

I believe, that in order to be able to recognize every use case of IDENTIFIER clause in the visitors I'd have to modify all the statements to provide access to TableFactor or some similar struct. This specific clause if problematic as it can appear anywhere an object name is referenced, and i believe there is no single place in the parser where i could add support for such a syntax. For now I am only interested in recognizing this clause in table names, so that limits the scope a little bit.

Does anyone have some ideas on how to best approach this problem? I'd like to use the limited resources I have in the best possible way and I feel like I'm missing something obvious here.

@iffyio, maybe you have some time and idea for that? You helped me with my previous PR 😄

Docs:
Snowflake
Databricks

@jmhain
Copy link
Contributor

jmhain commented Sep 5, 2024

I think the cleanest way would be to introduce an enum to replace ObjectName wherever an IDENTIFIER clause is allowed, something like:

enum ObjectNameOrIdentifierClause  {
    Name(ObjectName),
    IdentifierClause(IdentifierClause)
}

enum IdentifierClause {
    String(...),
    SessionVar(...),
    ...
}

@iffyio
Copy link
Contributor

iffyio commented Sep 6, 2024

Yeah I think @jmhain's suggestion to use the enum sounds good to support this!

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

No branches or pull requests

3 participants