Skip to content

NO-SNOW: deprecate disabling sql simplifer #4257

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.

Sign up for GitHub

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

Open
sfc-gh-aling wants to merge 1 commit into
base: main
Choose a base branch
from
Open

NO-SNOW: deprecate disabling sql simplifer #4257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 0 additions & 76 deletions .github/workflows/daily_modin_precommit_py310.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,82 +254,6 @@ jobs:
.tox/.coverage
.tox/coverage.xml

test-disable-sql-simplifier: # Will be removed after sql simplifier is stable and no option to opt out.
name: Test Disable SQL Simplifier modin-${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
runs-on: ${{ matrix.os.image_name }}
strategy:
fail-fast: false
matrix:
os:
- image_name: windows-latest-64-cores
download_name: windows
python-version: ["3.10"]
cloud-provider: [azure]
steps:
- name: Checkout Code
uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Set up uv
uses: astral-sh/setup-uv@v6
with:
version: "latest"
- name: Display Python version
run: python -c "import sys; print(sys.version)"
- name: Decrypt parameters.py
shell: bash
run: .github/scripts/decrypt_parameters.sh
env:
PARAMETER_PASSWORD: ${{ secrets.PARAMETER_PASSWORD }}
CLOUD_PROVIDER: ${{ matrix.cloud-provider }}
- name: Install protoc
shell: bash
run: .github/scripts/install_protoc.sh
- name: Download wheel(s)
uses: actions/download-artifact@v4
with:
name: wheel
path: dist
- name: Show wheels downloaded
run: ls -lh dist
shell: bash
- name: Upgrade setuptools, pip and wheel
run: uv pip install -U setuptools pip --system wheel
- name: Install tox
run: uv pip install tox --system
- if: ${{ contains('macos', matrix.os.download_name) }}
name: Run Snowpark pandas API doctests
run: python -m tox -e "py${PYTHON_VERSION}-doctest-snowparkpandasdoctest-modin-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short --disable_sql_simplifier
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Run Snowpark pandas API tests (excluding doctests)
run: python -m tox -e "py${PYTHON_VERSION/\./}-snowparkpandasdailynotdoctest-modin-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short --disable_sql_simplifier
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Combine coverages
run: python -m tox -e coverage --skip-missing-interpreters false
shell: bash
env:
SNOWFLAKE_IS_PYTHON_RUNTIME_TEST: 1
- uses: actions/upload-artifact@v4
with:
include-hidden-files: true
name: coverage_${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}-disable-sql-simplifier
path: |
.tox/.coverage
.tox/coverage.xml

test-disable-cte-optimization:
name: Test Disable CTE Optimization modin-${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
Expand Down
78 changes: 0 additions & 78 deletions .github/workflows/daily_precommit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,84 +270,6 @@ jobs:
.coverage
coverage.xml

test-disable-sql-simplifier: # Will be removed after sql simplifier is stable and no option to opt out.
name: Test Disable SQL Simplifier py-${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
runs-on: ${{ matrix.os.image_name }}
strategy:
fail-fast: false
matrix:
os:
- image_name: macos-latest
download_name: macos # it includes doctest
python-version: ["3.10", "3.11", "3.12"]
cloud-provider: [aws]
steps:
- name: Checkout Code
uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Set up uv
uses: astral-sh/setup-uv@v6
with:
version: "latest"
- name: Display Python version
run: python -c "import sys; print(sys.version)"
- name: Decrypt parameters.py
shell: bash
run: .github/scripts/decrypt_parameters.sh
env:
PARAMETER_PASSWORD: ${{ secrets.PARAMETER_PASSWORD }}
CLOUD_PROVIDER: ${{ matrix.cloud-provider }}
- name: Install protoc
shell: bash
run: .github/scripts/install_protoc.sh
- name: Download wheel(s)
uses: actions/download-artifact@v4
with:
name: wheel
path: dist
- name: Show wheels downloaded
run: ls -lh dist
shell: bash
- name: Upgrade setuptools, pip and wheel
run: uv pip install -U setuptools pip wheel --system
- name: Install tox
run: uv pip install tox --system
- if: ${{ contains('macos', matrix.os.download_name) }}
name: Run doctests
run: python -m tox -e "py${PYTHON_VERSION}-doctest-notudf-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short --disable_sql_simplifier
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Run tests (excluding doctests)
run: python -m tox -e "py${PYTHON_VERSION/\./}-dailynotdoctest-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short --disable_sql_simplifier
TOX_PARALLEL_NO_SPINNER: 1
SNOWPARK_PYTHON_API_TEST_BUCKET_PATH: ${{ secrets.SNOWPARK_PYTHON_API_TEST_BUCKET_PATH }}
SNOWPARK_PYTHON_API_S3_STORAGE_INTEGRATION: ${{ vars.SNOWPARK_PYTHON_API_S3_STORAGE_INTEGRATION }}
shell: bash
- name: Combine coverages
run: python -m tox -e coverage --skip-missing-interpreters false
shell: bash
env:
SNOWFLAKE_IS_PYTHON_RUNTIME_TEST: 1
- uses: actions/upload-artifact@v4
with:
include-hidden-files: true
name: coverage_${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}-disable-sql-simplifier
path: |
.tox/.coverage
.tox/coverage.xml

test-no-pandas:
name: Test when pandas is not installed py-${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#### Improvements

- Improved CTE optimization to deduplicate identical subtrees in self-joins, which were previously emitted as repeated subqueries.
- Added a warning when SQL simplifier is disabled (`session.sql_simplifier_enabled = False` or `PYTHON_SNOWPARK_USE_SQL_SIMPLIFIER = False`). The code path with SQL simplifier disabled is no longer maintained; if you encounter issues, please reach out through [GitHub issues](https://github.com/snowflakedb/snowpark-python/issues) or Snowflake support.

#### Deprecations

Expand Down
12 changes: 12 additions & 0 deletions src/snowflake/snowpark/_internal/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import threading
import traceback
import uuid
import warnings
import zipfile
from enum import Enum, IntEnum, auto, unique
from functools import lru_cache, wraps
Expand Down Expand Up @@ -454,6 +455,17 @@ def warn_session_config_update_in_multithreaded_mode(config: str) -> None:
)


def warn_sql_simplifier_disabled(stacklevel: int = 2) -> None:
warnings.warn(
"SQL simplifier is disabled (sql_simplifier_enabled=False). This code path "
"is no longer maintained. If you encounter issues, please reach out through "
"GitHub issues (https://github.com/snowflakedb/snowpark-python/issues) or "
"Snowflake support.",
UserWarning,
stacklevel=stacklevel,
)


def normalize_remote_file_or_dir(name: str) -> str:
return normalize_path(name, is_local=False)

Expand Down
5 changes: 5 additions & 0 deletions src/snowflake/snowpark/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
unwrap_stage_location_single_quote,
validate_object_name,
warn_session_config_update_in_multithreaded_mode,
warn_sql_simplifier_disabled,
warning,
zip_file_or_directory_to_stream,
set_ast_state,
Expand Down Expand Up @@ -659,6 +660,8 @@ def __init__(
_PYTHON_SNOWPARK_USE_SQL_SIMPLIFIER_STRING, True
)
)
if not self._sql_simplifier_enabled:
warn_sql_simplifier_disabled(stacklevel=3)
self._cte_optimization_enabled: bool = self.is_feature_enabled_for_version(
_PYTHON_SNOWPARK_USE_CTE_OPTIMIZATION_VERSION
)
Expand Down Expand Up @@ -1148,6 +1151,8 @@ def custom_package_usage_config(self) -> Dict:
@sql_simplifier_enabled.setter
def sql_simplifier_enabled(self, value: bool) -> None:
warn_session_config_update_in_multithreaded_mode("sql_simplifier_enabled")
if not value and value != self._sql_simplifier_enabled:
warn_sql_simplifier_disabled()

with self._lock:
self._conn._telemetry_client.send_sql_simplifier_telemetry(
Expand Down
Loading