Skip to content

Commit

Permalink
SNOW-1842459 Fix line no issues in ast expectation validation tests. (#…
Browse files Browse the repository at this point in the history 
…2700)

1. Which Jira issue is this PR addressing? Make sure that there is an
accompanying issue to your PR.

Fixes[SNOW-1842459](https://snowflakecomputing.atlassian.net/browse/SNOW-1842459)

2. Fill out the following pre-review checklist:

- [ ] I am adding a new automated test(s) to verify correctness of my
new code
- [ ] If this test skips Local Testing mode, I'm requesting review from
@snowflakedb/local-testing
- [ ] I am adding new logging messages
- [ ] I am adding a new telemetry message
- [ ] I am adding new credentials
- [ ] I am adding a new dependency
- [ ] If this is a new feature/behavior, I'm adding the Local Testing
parity changes.
- [x] I acknowledge that I have ensured my changes to be thread-safe.
Follow the link for more information: [Thread-safe Developer
Guidelines](https://github.com/snowflakedb/snowpark-python/blob/main/CONTRIBUTING.md#thread-safe-development)

3. Please describe how your code solves the related issue.

The frame_info line information differs across python versions for
chained operations, so to mitigate we only compare line no information
for python 3.9 and not other versions.
  • Loading branch information
@sfc-gh-evandenberg
sfc-gh-evandenberg authored Dec 3, 2024
1 parent 769e7f6 commit 2a45e4b
Show file tree
Hide file tree
Showing 111 changed files with 15,969 additions and 60 deletions.
25 changes: 25 additions & 0 deletions src/snowflake/snowpark/_internal/ast/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import dateutil
from dateutil.tz import tzlocal
from google.protobuf.text_format import MessageToString, Parse
from google.protobuf.message import Message

import snowflake.snowpark
import snowflake.snowpark._internal.proto.generated.ast_pb2 as proto
Expand Down Expand Up @@ -1494,3 +1495,27 @@ def base64_lines_to_textproto(base64_str: str) -> str:
def textproto_to_request(textproto_str: str) -> proto.Request:
request = Parse(textproto_str, proto.Request())
return request


def clear_line_no_in_ast(ast: Any) -> None:
"""Clear any 'src' information in the statement body."""
if isinstance(ast, Iterable) and not isinstance(ast, str):
for c in ast:
clear_line_no_in_ast(c)
elif hasattr(ast, "DESCRIPTOR"):
if hasattr(ast, "src"):
ast.ClearField("src") # type: ignore[union-attr]

for f in ast.DESCRIPTOR.fields:
c = getattr(ast, f.name)
if (isinstance(c, Iterable) and not isinstance(c, str) and len(c) > 0) or ( # type: ignore[arg-type]
isinstance(c, Message) and c.ByteSize() > 0
):
clear_line_no_in_ast(c)


def clear_line_no_in_request(request: proto.Request) -> None:
"""There are inconsistencies in the frame_info.line_no depending on the python version, this seems to be due to
fixes in determining better line_no info for chained python code, etc."""
for stmt in request.body:
clear_line_no_in_ast(stmt)
2 changes: 2 additions & 0 deletions tests/ast/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ All generated AST should be tested using this mechanism. To add a test, create a

N.B. No eager evaluation is permitted, as any intermediate batches will not be observed. This can easily be changed if necessary, however.

The src line no information is only captured when run with python 3.11+ because of bug fix/differences in frame line no across earlier python versions.

```python
## TEST CASE

Expand Down
90 changes: 90 additions & 0 deletions tests/ast/data/DataFrame.agg.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,19 @@ body {
vs {
list_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 1
Expand All @@ -46,7 +52,10 @@ body {
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 2
Expand All @@ -57,13 +66,19 @@ body {
vs {
list_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 3
Expand All @@ -72,7 +87,10 @@ body {
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 4
Expand All @@ -83,13 +101,19 @@ body {
vs {
list_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 1
Expand All @@ -98,7 +122,10 @@ body {
vs {
int64_val {
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
v: 4
Expand All @@ -115,7 +142,10 @@ body {
}
}
src {
end_column: 82
end_line: 27
file: "SRC_POSITION_TEST_MODE"
start_column: 13
start_line: 27
}
}
Expand Down Expand Up @@ -166,20 +196,29 @@ body {
pos_args {
string_val {
src {
end_column: 36
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 28
start_line: 29
}
v: "a"
}
}
src {
end_column: 36
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 28
start_line: 29
}
}
}
src {
end_column: 37
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 21
start_line: 29
}
}
Expand Down Expand Up @@ -209,28 +248,40 @@ body {
pos_args {
string_val {
src {
end_column: 58
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 50
start_line: 29
}
v: "a"
}
}
src {
end_column: 58
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 50
start_line: 29
}
}
}
src {
end_column: 59
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 39
start_line: 29
}
}
}
variadic: true
}
src {
end_column: 60
end_line: 29
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 29
}
}
Expand Down Expand Up @@ -259,13 +310,19 @@ body {
args {
tuple_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
vs {
string_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
v: "a"
Expand All @@ -274,7 +331,10 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
v: "min"
Expand All @@ -285,13 +345,19 @@ body {
args {
tuple_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
vs {
string_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
v: "b"
Expand All @@ -300,7 +366,10 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
v: "max"
Expand All @@ -311,7 +380,10 @@ body {
variadic: true
}
src {
end_column: 48
end_line: 31
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 31
}
}
Expand Down Expand Up @@ -343,7 +415,10 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
v: "a"
Expand All @@ -352,7 +427,10 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
v: "count"
Expand All @@ -363,7 +441,10 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
v: "b"
Expand All @@ -372,23 +453,32 @@ body {
vs {
string_val {
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
v: "sum"
}
}
}
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
}
}
variadic: true
}
src {
end_column: 48
end_line: 33
file: "SRC_POSITION_TEST_MODE"
start_column: 14
start_line: 33
}
}
Expand Down
Loading

0 comments on commit 2a45e4b

Please sign in to comment.