Snowflake Dialect and Testing #363

hadia206 · 2025-06-13T17:39:04Z

resolves #296

Add Snowflake dialect support
Add SF dialect tests for TPC-H and Defog
Update dialect bindings based on available Snowflake functions.
Update PR CI to include flag for testing all dialects.
Update Defog init scripts in different dialects to match original scripts + add one for Snowflake.

All dialects testing passed, see here

knassre-bodo

Left behind some initial feedback

.github/workflows/sf_testing.yml

tests/conftest.py

pydough/sqlglot/execute_relational.py

…ure semi

knassre-bodo

Few more revisions

pydough/sqlglot/transform_bindings/sf_transform_bindings.py

pyproject.toml

tests/conftest.py

tests/test_pipeline_tpch.py

tests/conftest.py

README.md

review-notebook-app · 2025-06-19T23:27:55Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

…dia/sf

knassre-bodo · 2025-08-26T15:54:13Z

README.md

+When submitting a PR, you can control which CI tests run by adding special flags
+to your **latest commit message**.


Let's specify that all of these flags are case insensitive

knassre-bodo · 2025-08-26T15:54:50Z

README.md

+When submitting a PR, you can control which CI tests run by adding special flags
+to your **latest commit message**.
+
+- To run **PyDough CI tests**, add: `[run CI]`  


Specify that this one will not run tests from any sql database except sqlite

knassre-bodo · 2025-08-26T16:00:20Z

demos/notebooks/SF_TPCH_q1.ipynb

@@ -0,0 +1,307 @@
+{


Let's also show another example where we create the connector object, THEN pass it in to Snowflake with connection=..., then run another test on that (& after that test show us doing something with the connector, like fetching the query text of the last query run through that connector).

Reply via ReviewNB

knassre-bodo · 2025-08-26T16:00:20Z

demos/notebooks/SF_TPCH_q1.ipynb

@@ -0,0 +1,307 @@
+{


NIT: let's fix the formatting on the first line so lines is on the next line and is matched up with .PARTITION

Reply via ReviewNB

knassre-bodo

LGTM, just some last nitpicks and cleanup revisions

knassre-bodo · 2025-08-26T16:04:23Z

pydough/sqlglot/execute_relational.py

+        "validate_qualify_columns": False,
+    }
+    # Exclude Snowflake dialect to avoid some issues
+    # related to qualify and column decorrelation


Is it column de-correlation or just column name qualification? I ask because I'm pretty sure qualification happens somewhere else

You're right. I mixed things up.
updated it

knassre-bodo · 2025-08-26T16:07:07Z

tests/conftest.py

+@pytest.fixture(scope="session")
+def get_sf_sample_graph(
+    sf_sample_graph_path: str,
+    valid_sample_graph_names: set[str],
+) -> graph_fetcher:
+    """
+    A function that takes in the name of a graph from the supported sample
+    Snowflake graph names and returns the metadata for that PyDough graph.
+    """
+
+    @cache
+    def impl(name: str) -> GraphMetadata:
+        if name not in valid_sample_graph_names:
+            raise Exception(f"Unrecognized graph name '{name}'")
+        return pydough.parse_json_metadata_from_file(
+            file_path=sf_sample_graph_path, graph_name=name
+        )
+
+    return impl
+
+
+@pytest.fixture(scope="session")
+def get_sf_defog_graphs() -> graph_fetcher:


Let's merge these: we can place the sample graphs & defog graphs for Snowflake in the same JSON file, and merge the fixtures since they are functions that take in the name of a graph and return that graph within that file. We can ignore the valid_sample_graph_names stuff since that is a mostly-redundant layer of error checking.

I'll address that in a followup PR. For now, I will keep them separate to match what we have for SQLite.

knassre-bodo · 2025-08-26T16:08:45Z

pydough/sqlglot/sqlglot_helpers.py

    return expr.this if isinstance(expr, SQLGlotAlias) else expr
+
+
+def is_boolean_expression(expr: SQLGlotExpression) -> bool:


Is this being used anywhere? I can't find it. If not, let's delete.

Good catch, old code

knassre-bodo · 2025-08-26T16:12:03Z

pydough/sqlglot/transform_bindings/sf_transform_bindings.py

+    def convert_get_part(
+        self, args: SQLGlotExpression, types: list[PyDoughType]
+    ) -> SQLGlotExpression:
+        return sqlglot_expressions.Anonymous(this="SPLIT_PART", expressions=args)


Can get rid of this if we are using PYDOP_TO_SNOWFLAKE_FUNC

knassre-bodo · 2025-08-26T16:13:13Z

pydough/sqlglot/transform_bindings/sf_transform_bindings.py

+    def convert_sum(
+        self, arg: SQLGlotExpression, types: list[PyDoughType]
+    ) -> SQLGlotExpression:


I have an idea: let's move this method to the base class, then have this be the override, that way we don't need to case on SUM in the subclasses' implementation of convert_call_to_sqlglot. We can also edit the mysql file accordingly.

Updated base and SF files.
MySQL doesn't have SUM variant

🤦 I was thinking of PostgreSQL

knassre-bodo · 2025-08-26T16:13:41Z

pydough/sqlglot/transform_bindings/sf_transform_bindings.py

+            arg (SQLGlotExpression): The argument to the SUM function.
+            types (list[PyDoughType]): The types of the arguments.


Let's include backticks for better tooltips, and exclude the types (since they are already in the type annotation).

Suggested change

arg (SQLGlotExpression): The argument to the SUM function.

types (list[PyDoughType]): The types of the arguments.

`arg` The argument to the SUM function.

`types`: The types of the arguments.

john-sanchez31 · 2025-08-26T15:36:45Z

pydough/sqlglot/override_simplify.py

+    constant_propagation: bool = False,
+    dialect: DialectType = None,
+    max_depth: int | None = None,
+):


Add type hint for the return and throughout the function if it's necessary

copy/paste from SqlGlot so won't update that

john-sanchez31 · 2025-08-26T15:37:07Z

pydough/sqlglot/override_simplify.py

+    Args:
+        expression: expression to simplify
+        constant_propagation: whether the constant propagation rule should be used
+        max_depth: Chains of Connectors (AND, OR, etc) exceeding `max_depth` will be skipped


Add the dialect parameter

john-sanchez31 · 2025-08-26T15:37:38Z

pydough/sqlglot/override_simplify.py

+        constant_propagation: whether the constant propagation rule should be used
+        max_depth: Chains of Connectors (AND, OR, etc) exceeding `max_depth` will be skipped
+    Returns:
+        sqlglot.Expression: simplified expression


Suggested change

sqlglot.Expression: simplified expression

simplified expression

john-sanchez31 · 2025-08-26T15:58:15Z

pydough/sqlglot/sqlglot_helpers.py

+    if isinstance(expr, boolean_types):
+        return True
+
+    return False


We can simplify this just to:

Suggested change

if isinstance(expr, boolean_types):

return True

return False

return isinstance(expr, boolean_types)

Thanks for the tip. However, whole function not needed so deleted that code.

john-sanchez31 · 2025-08-26T16:29:25Z

pydough/sqlglot/transform_bindings/sf_transform_bindings.py

+        unit: DateTimeUnit,
+    ) -> SQLGlotExpression:
+        # Update argument type to fit datetime
+        dt_expr = self.handle_datetime_base_arg(args[0])


Type hint if required

knassre-bodo reviewed Jun 16, 2025

View reviewed changes

.github/workflows/sf_testing.yml Outdated Show resolved Hide resolved

.github/workflows/sf_testing.yml Outdated Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

knassre-bodo reviewed Jun 16, 2025

View reviewed changes

pydough/sqlglot/execute_relational.py Outdated Show resolved Hide resolved

knassre-bodo added 4 commits June 17, 2025 12:22

Enable de-correlation of pure semi/anti joins, including pullup for p…

6abec11

…ure semi

Fix correl 16 WIP

9b4fb04

Fixed bugs, need to deal with window squished CTEs for correl #16

319797f

Merge branch 'main' into kian/decor_semi_anti

634585c

knassre-bodo reviewed Jun 19, 2025

View reviewed changes

Resolving conflicts [RUN CI]

6361404

hadia206 force-pushed the Hadia/sf branch from 22a8aeb to 4edb592 Compare June 19, 2025 23:06

hadia206 changed the base branch from main to kian/decor_semi_anti June 19, 2025 23:07

hadia206 changed the base branch from kian/decor_semi_anti to main June 20, 2025 14:10

Squash merge Hadia/sf changes onto kian/decor_semi_anti

e4764ca

hadia206 force-pushed the Hadia/sf branch from e69eed7 to e4764ca Compare June 20, 2025 14:56

hadia206 changed the base branch from main to kian/decor_semi_anti June 20, 2025 14:57

hadia206 added 3 commits June 20, 2025 07:58

[run SF]

8dbcd9c

[run SF] add connection object as option for load_snowflake_connection

0c3167e

Use SF Functions directly instead of PyDough changes (#377)

19baea0

Base automatically changed from kian/decor_semi_anti to main June 23, 2025 15:44

hadia206 added 11 commits July 7, 2025 13:29

Merge branch 'main' of https://github.com/bodo-ai/PyDough into Hadia/sf

716f1aa

[run CI] [run SF] merge main and re-run tests

f5c80b4

Merge branch 'Hadia/sf' of https://github.com/bodo-ai/PyDough into Ha…

f6e1f5e

…dia/sf

[run CI] [run SF] merge and re-run tests

e0a135d

merge

597f5f9

update tests and fix some SF issues

95895c8

add new test files

a2c4d1f

update dayname

7fbcdc6

fix date_trunc

0a8ac03

[run SF] [run CI] handle change in start of week

1b6a84b

comment

15c0242

hadia206 added 14 commits August 21, 2025 14:24

fix date_trunc in sqlite and uncomment row

a703b41

add run-all option

8c40bac

mysql in dev dependency

296f0c1

add connectors to docs

dc06546

move week7 in week_offset

55545c2

update try/except for mysql

1558b7c

move week7 in week_offset

377b523

remove str_count

fcc698b

[run all] update mysql init script

ae623b3

undo changes for sql text

3de8b89

modify week_offset to be like mysql for consistency

ed8f82e

modify week_offset to be like mysql for consistency

1c33249

[run all] disable dialect for SF only

5f31407

[run all] fix link and remove old test

8a28f4a

hadia206 changed the title ~~[DRAFT] SF Testing and Dialect~~ Snowflake Dialect and Testing Aug 25, 2025

hadia206 marked this pull request as ready for review August 25, 2025 23:39

hadia206 requested review from a team, john-sanchez31, juankx-bodo and knassre-bodo and removed request for a team August 25, 2025 23:39

knassre-bodo reviewed Aug 26, 2025

View reviewed changes

knassre-bodo approved these changes Aug 26, 2025

View reviewed changes

john-sanchez31 reviewed Aug 26, 2025

View reviewed changes

hadia206 and others added 3 commits August 26, 2025 14:09

address comments [run all]

35c1c2e

Fixing MySQL testing issues [RUN ALL]

0d6eddf

Fixing start of week problems in MySQL setup script [RUN ALL]

45b43de

hadia206 merged commit ebc0e40 into main Aug 26, 2025
12 checks passed

hadia206 deleted the Hadia/sf branch August 26, 2025 23:20

		When submitting a PR, you can control which CI tests run by adding special flags
		to your latest commit message.

		return expr.this if isinstance(expr, SQLGlotAlias) else expr


		def is_boolean_expression(expr: SQLGlotExpression) -> bool:

		arg (SQLGlotExpression): The argument to the SUM function.
		types (list[PyDoughType]): The types of the arguments.

	sqlglot.Expression: simplified expression
	simplified expression

Snowflake Dialect and Testing #363

Snowflake Dialect and Testing #363

Uh oh!

Conversation

hadia206 commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knassre-bodo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knassre-bodo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

review-notebook-app bot commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knassre-bodo Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knassre-bodo Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knassre-bodo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hadia206 commented Jun 13, 2025 •

edited

Loading

knassre-bodo Aug 26, 2025 •

edited

Loading

knassre-bodo Aug 26, 2025 •

edited

Loading