Skip to content

SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main) - #4307

Open
sfc-gh-fpawlowski wants to merge 2 commits into
mainfrom
create-temp-table-ast-fix-on-main
Open

SNOW-2912540: translate create_temp_table=True to table_type in AST encoding (rebased onto main)#4307
sfc-gh-fpawlowski wants to merge 2 commits into
mainfrom
create-temp-table-ast-fix-on-main

Conversation

@sfc-gh-fpawlowski

@sfc-gh-fpawlowski sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown

Summary

Rebases the fix from #4296 onto main. #4296's head branch (worktree-create-temp-table-ast-fix) is based on the stale ud-local-test-scripts branch, which is 49 commits behind main — so it can't land there cleanly. This PR cherry-picks #4296's two commits (unchanged, same authorship) directly onto current main, where they apply without conflicts.

Same fix as #4296: stops emitting the deprecated create_temp_table parameter into the encoded AST sent to the server, without changing Snowpark's own public API — create_temp_table stays in the function signatures, still fires its deprecation warning, and still works exactly as before for callers. Only table_type (already resolved from create_temp_table when needed) is now recorded in the AST.

Changes:

  • dataframe_writer.py: Move create_temp_table deprecation coercion to before the AST emission block in save_as_table, so table_type is already resolved when WriteTable is encoded. Remove expr.create_temp_table emission.
  • session.py: Remove ast.create_temp_table = create_temp_table from the write_pandas AST block — the coercion already fires before AST emission there, so ast.table_type carries the correct value.
  • dataframe.py: Replace create_temp_table=True with table_type="temp" in the internal cache_result mock path, matching the real code path and avoiding a spurious deprecation warning from internal code.
  • ast.proto: Mark both create_temp_table fields as // Deprecated: use table_type instead. (fields retained for wire compatibility).
  • tests/ast/data/session_write_pandas.test: Remove create_temp_table: true from expected encoded AST and create_temp_table=True from expected unparser output.

Test plan

  • tests/ast/test_ast_driver.py::test_ast[session_write_pandas.test] passes
  • tests/ast/test_ast_driver.py::test_ast[DataFrame.write.test] passes
  • Manually verified on a local-testing session: save_as_table(..., create_temp_table=True) still logs the deprecation warning and creates a temp table
  • Manually verified cache_result() no longer emits the spurious deprecation warning

Supersedes #4296 for the purpose of landing on main.

Checklist

  • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support.
  • I acknowledge that I have ensured my changes to be thread-safe

Stack (via Graphite)

🤖 Generated with Claude Code

@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (6ab27ed) to head (0ce0042).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4307      +/-   ##
==========================================
- Coverage   95.24%   95.24%   -0.01%     
==========================================
  Files         171      171              
  Lines       44720    44718       -2     
  Branches     7676     7676              
==========================================
- Hits        42593    42591       -2     
  Misses       1340     1340              
  Partials      787      787              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sfc-gh-fpawlowski and others added 2 commits August 20, 2026 05:51
…ncoding

The deprecated `create_temp_table` parameter was being emitted to the
proto AST as a separate boolean field even though the runtime already
translates it to `table_type="temporary"`. This meant the AST decoder
had to handle two representations for the same thing.

Fix: move the deprecation coercion before the AST block in save_as_table
so `table_type` is already resolved when emitted; remove the deprecated
field from both AST emission sites (WriteTable and WritePandas); update
the internal cache_result mock path to pass table_type="temp" directly;
mark the proto fields as deprecated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_table removal

Remove create_temp_table from the expected encoded AST and unparser
output in the write_pandas golden test — the field is no longer emitted
to the proto since the deprecation coercion now happens before the AST
block, making table_type the sole carrier of this information.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants