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

[125] Fix identifier issue #126

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

dwreeves
Copy link
Contributor

Resolves #125.

  • Strips return type from FQN (note: I do not know if this is something that should be done? Help me out.)
  • Handles quoted functions

So the following identifier: a.b."c(x varchar):varchar(12345)" returns a.b."c" as the scoped name and x varchar as the arg types, and handles quotes properly and also strips the return type.

However, a limitation of this is that it does not support colons in double quoted identifiers, e.g.

create view foo ("weird:column": varchar) as ... will fail here.

@dwreeves dwreeves force-pushed the 125-fix-parse-identifier branch from 8b16ff1 to e5fee34 Compare October 17, 2024 23:44
@dwreeves dwreeves had a problem deploying to snowflake-aws-enterprise October 17, 2024 23:45 — with GitHub Actions Failure
@dwreeves dwreeves had a problem deploying to snowflake-aws-standard October 17, 2024 23:45 — with GitHub Actions Failure
@dwreeves
Copy link
Contributor Author

FWIW, this error:

ValueError: 'int' is not callable

Is caused by the most recent version of pytest-profiling: man-group/pytest-plugins#242 I am updating the setup.py to be !=1.8.0

@dwreeves
Copy link
Contributor Author

Also, tested the code changes on my own Snowflake deployment.

It works in the sense that it avoids a parsing error, but I do not know enough about the library to know whether this method of parsing is "good." I also don't want to have to deal with the problem of handling quoted identifiers that use weird characters (colons specifically), to be honest...

@dwreeves dwreeves had a problem deploying to snowflake-aws-standard October 17, 2024 23:52 — with GitHub Actions Failure
@dwreeves dwreeves had a problem deploying to snowflake-aws-enterprise October 17, 2024 23:52 — with GitHub Actions Failure
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
titan/identifiers.py 50.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
setup.py 0.00% <ø> (ø)
titan/identifiers.py 82.64% <50.00%> (-5.29%) ⬇️

... and 40 files with indirect coverage changes

@dwreeves dwreeves had a problem deploying to snowflake-aws-standard October 17, 2024 23:54 — with GitHub Actions Failure
@dwreeves dwreeves had a problem deploying to snowflake-aws-enterprise October 17, 2024 23:54 — with GitHub Actions Error
@teej
Copy link
Collaborator

teej commented Oct 18, 2024

Integration tests are being flaky, I ran them myself and it was all green. LGTM

@teej teej merged commit 6cf51e2 into Titan-Systems:main Oct 18, 2024
3 of 6 checks passed
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.

parse_identifier fails on quoted UDF
2 participants