-
Notifications
You must be signed in to change notification settings - Fork 120
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
Unify Snowflake object name handling in the Snowpark AST #2789
Conversation
The serialized batch was getting large enough to exceed command-line length limits
expr_builder.sp_table_name_structured.name.extend(name) | ||
else: | ||
raise ValueError(f"Invalid name type {type(name)} for SpTableName entity.") | ||
def build_sp_table_name( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these type-specific functions necessary? One thing I notice is that the re-emitted exceptions lose the diagnostic contents of the original exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I notice is that the re-emitted exceptions lose the diagnostic contents of the original exception.
That's weird, raise ... from ...
should preserve the contents. The functions aren't strictly necessary, but I find the call sites more readable (the name clarifies the intent), and the errors should have been more readable, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1621205
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This is the client-side change corresponding to https://github.com/snowflakedb/snowflake/pull/240557
Unify Snowflake object name handling in the Snowpark AST.
Remove
FnName
andSpTableName
. They both hadFlat
andStructured
variants, but ultimately designate Snowflake object names.Introduce
data SpName
andentity SpNameRef
for referring to Snowflake objects by relative or fully qualified name.Update
FnNameRefExpr
.Use
SpNameRef
in a few places that used to useList[String]
.