[feature branch] AST test infra #1701
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sfc-gh-azwiegincew
requested review from
sfc-gh-lspiegelberg,
sfc-gh-heshah,
sfc-gh-vbudati and
sfc-gh-oplaton
sfc-gh-azwiegincew
requested review from
sfc-gh-smirzaei,
sfc-gh-yuwang and
sfc-gh-aalam
and removed request for
a team
sfc-gh-azwiegincew
removed request for
sfc-gh-smirzaei,
sfc-gh-yuwang,
sfc-gh-aalam and
a team
|
Can we maybe avoid checking in the .jar file? It's 22MB which will make any changesets large... |
| @@ -1133,7 +1133,8 @@ def select( | |||
|
|
|||
| stmt = self._session._ast_batch.assign() | |||
| ast = stmt.expr | |||
| ast.sp_dataframe_select__columns.df.sp_dataframe_ref.id.bitfield1 = self._ast_id | |||
| # TODO: remove the None guard below once we generate the correct AST. | |||
There was a problem hiding this comment.
what is 666 about? I assume just some random id for now?
There was a problem hiding this comment.
Yeah. Once we get full coverage of the API, this won't be necessary to make the mock setup pass.
| @@ -795,6 +795,9 @@ def get_result_query_id(self, plan: SnowflakePlan, **kwargs) -> str: | |||
| # get the iterator such that the data is not fetched | |||
| result_set, _ = self.get_result_set(plan, to_iter=True, **kwargs) | |||
| return result_set["sfqid"] | |||
|
|
|||
| def create_coprocessor(self): | |||
There was a problem hiding this comment.
would we add in the future any functionality to this? Or is this a simple stub? In any case maybe a short comment to address either 1.) what needs to get added here or 2.) that this should not be removed, but left because of ... would help :)
sfc-gh-lspiegelberg
approved these changes
May 30, 2024
sfc-gh-azwiegincew
merged commit 037fc60
into
server-side-snowpark
6 of 8 checks passed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AST Tests
This driver enables testing of the AST generation that will be used in the server-side Snowpark implementation, starting with Phase 0.
All generated AST should be tested using this mechanism. To add a test, create a new file under
tests/ast/data. Files look like the following example. The test driver sets up the session and looks at the accumulated lazy values in the resulting environment.N.B. No eager evaluation is permitted, as any intermediate batches will not be observed. This can easily be changed if necessary, however.
To generate the expected output the first time the test is run, or when the AST generation changes, run: