Add support for Trino dialect and database context in PyDough by knassre-bodo · Pull Request #498 · bodo-ai/PyDough

knassre-bodo · 2026-03-04T02:57:55Z

Adding support for the Trino dialect in Pydough, including testing with a docker container that has four Trino connections to other databases: MySQL, Postgres, Mongo, and Cassandra.

…n modified tests

for more information, see https://pre-commit.ci

john-sanchez31

Great job Kian! There are some comments below but overall almost ready

john-sanchez31 · 2026-05-11T14:41:49Z

+        image: bodoai1/pydough-mongo-tpch:latest
+        env:
+          # Set environment variables for MongoDB container
+          MONGO_HOST: 127.0.0.1


I think It'd be better to create the corresponding environment variables for mongo

john-sanchez31 · 2026-05-11T14:43:14Z

+            MYSQL_USER: ${{ secrets.MYSQL_USERNAME }}
+            MYSQL_PASSWORD: ${{ secrets.MYSQL_PASSWORD }}
+            MONGO_HOST: mongo
+            MONGO_USER: "root_user"


Those can be reused here (avoiding typing errors)

+1. Moving to variables an re-using

john-sanchez31 · 2026-05-11T16:03:36Z

    with ID 1? Return the coupon ID and total amount transacted with it.
    """
-    return coupons.WHERE(merchant_id == "1").CALCULATE(
+    return coupons.WHERE(merchant_id == 1).CALCULATE(


Why this change? Does it work for all dialects? I know some (Oracle) might have trouble handling this change.

It technically is SUPPOSED to be an integer; I don't know why it was ever a string, but doing so was causing problems since comparing integers to strings in Trino is problematic. Rather than adding implicit type casting, it was simpler to just fix the test so it made more sense.

john-sanchez31 · 2026-05-11T16:22:15Z

 FROM tpch.lineitem
 ORDER BY
-  1 NULLS FIRST
+  l_discount NULLS FIRST


Why is it using the actual column name instead of the index?

The actual test here was reworked so it orders it in a manner where the only row kept is the one where l_discount is 0.

john-sanchez31 · 2026-05-11T20:42:00Z

+                        "-e",
+                        "TPCH_TABLES=region,supplier",
+                        "-e",
+                        "DEFOG_TABLES=sbCustomer,salespersons,payments_received,doctors,diagnoses,concomitant_meds,merchants,wallet_user_balance_daily,user_sessions,cite,domain_author,domain_keyword,keyword,publication_keyword,location",


Why is this needed? When more defog db are added does this need to be updated?

Why is this needed?

Mongo/Cassandra setup time scales a lot with the number of tables. Only having it set up some of them (the ones that Trino will use from these connectors), instead of all of them makes it faster.

When more defog db are added does this need to be updated?

Yes, unless we choose to make it so those defog db are only ever accessed from the MySQL/Postgres connectors.

john-sanchez31 · 2026-05-11T20:43:26Z

+            client = MongoClient(
+                host=MONGO_HOST,
+                port=MONGO_PORT,
+                username="root_user",


Maybe would be better use environment variables. I know the data in the container are not sensitive but still would be a better practice and for consistency.

john-sanchez31 · 2026-05-11T20:45:27Z

    return request.param


+@pytest.mark.custom_datasets


I agree, needed sooner or later

john-sanchez31 · 2026-05-11T21:14:19Z

+
+
+@pytest.mark.skip(
+    "Trino does not properly handle all of the weird quoting cases; investigate at a later time."


We can leave it like this but just take in count that other type of tests can be added as custom_datasets not just the quoted ones.

hadia206

Good work, Kian!
Please see my comments below.

Also, don't forget the documentation.

hadia206 · 2026-05-12T18:01:24Z

+            MYSQL_USER: ${{ secrets.MYSQL_USERNAME }}
+            MYSQL_PASSWORD: ${{ secrets.MYSQL_PASSWORD }}
+            MONGO_HOST: mongo
+            MONGO_USER: "root_user"


+1. Moving to variables an re-using

hadia206 · 2026-05-12T18:01:58Z

+        run: uv venv
+
+      - name: Install dependencies
+        run: uv pip install -e ".[trino]"; uv pip install pytest


Do we still need the uv pip install pytest?

This is the only/main part still under development

hadia206 · 2026-05-12T18:05:44Z

-            return parse_one(combined_string)
+            return parse_one(
+                combined_string,
+                dialect=self._visitor._session.database.dialect.sqlglot_dialect,


Why this change?

Discovered the hard way while doing some Trino UDF stuff that the UDF will not always be parsed by SQLGlot correctly unless the dialect is passed in. E.g. if I use DATE_ADD inside the SQL macro text, SQLGlot parses the arguments in the wrong order unless the dialect was provided.

hadia206 · 2026-05-12T18:10:06Z

+            cast of the original expression to string.
+        """
+        if not isinstance(typ, StringType):
+            return sqlglot_expressions.Cast(this=expr, to="VARCHAR")


Shouldn't this be

Suggested change

return sqlglot_expressions.Cast(this=expr, to="VARCHAR")

return sqlglot_expressions.Cast(this=expr, to=sqlglot_expressions.DataType.build("VARCHAR"))

hadia206 · 2026-05-12T18:12:52Z

+        return super().convert_call_to_sqlglot(operator, args, types)
+
+    def convert_sum(
+        self, arg: SQLGlotExpression, types: list[PyDoughType]


Suggested change

self, arg: SQLGlotExpression, types: list[PyDoughType]

self, args: list[SQLGlotExpression], types: list[PyDoughType]

Then update use of arg to args

hadia206 · 2026-05-12T18:48:18Z

+        num_term = float(numerator.this)
+        denom_term = float(denominator.this)
+        if denom_term != 0 and num_term / denom_term == int(num_term / denom_term):
+            return exp.Literal.number(int(num_term / denom_term))


Suggested by AI.
Reason:
sqlglot.expressions.Div can represent either integer (//) or float (/) division depending on context. Replacing 4 / 2 with the integer literal 2 silently changes the type from float to int for float-division contexts.
Additionally, the equality check float == int can produce false positives for very large numbers due to floating-point representation (e.g., 10**16 + 1 / 1 would fold incorrectly). Using num_term % denom_term == 0 with integer arithmetic (int(num_term) and int(denom_term)) where both values are whole numbers would be safer.

Suggested change

num_term = float(numerator.this)

denom_term = float(denominator.this)

if denom_term != 0 and num_term / denom_term == int(num_term / denom_term):

return exp.Literal.number(int(num_term / denom_term))

num_term = int(float(numerator.this))

denom_term = int(float(denominator.this))

if denom_term != 0 and num_term % denom_term == 0:

return exp.Literal.number(num_term // denom_term)

Will need to verify that both terms are actually integers... e.g. I don't want to have 6.5 / 3.2 get folded into 3

Did you verify? If yes, what's final decision?

Changed it to this:

# Attempt to fold constants for cases like `4 / 2` -> `2` if ( isinstance(numerator, exp.Literal) and numerator.is_int and isinstance(denominator, exp.Literal) and denominator.is_int ): num_term: int = numerator.this denom_term: int = denominator.this if denom_term != 0 and num_term % denom_term == 0: return exp.Literal.number(num_term // denom_term)

hadia206 · 2026-05-12T18:54:29Z

+            # correctly whereas the SQLGlot version gets confused with multiple
+            # rounds of parsing and unparsing.
+            Trino.Generator.TRANSFORMS[sqlglot_expressions.DayOfWeek] = (
+                lambda self, e: f"(({self.func('DAY_OF_WEEK', e.this)} % 7) + 1)",


Assuming it's a typo. There was no , in the case written beforehand.

Suggested change

lambda self, e: f"(({self.func('DAY_OF_WEEK', e.this)} % 7) + 1)",

lambda self, e: f"(({self.func('DAY_OF_WEEK', e.this)} % 7) + 1)"

hadia206 · 2026-05-12T19:05:08Z

+def get_table_prefix_for_dialect(dialect: DatabaseDialect) -> str:
+    """Return the appropriate table name prefix for the given database dialect."""
+    match dialect:
+        case DatabaseDialect.SNOWFLAKE:


Let's re-add comment here in docstring or above each case.

# For Snowflake, use cross-database write (read from SNOWFLAKE_SAMPLE_DATA, # write to E2E_TESTS_DB.PUBLIC)

hadia206 · 2026-05-12T19:07:28Z

    )


-def simple_week_sampler():


Why was this removed? I see the tpc-h one all combined into one. but this one I don't see it re-added somewhere else or I may have missed it.

It was all combined into one in "test_pipeline_e2e_simple_week", which uses the TPC-H version.

john-sanchez31

Great job Kian! Ready to be merged after CI is working

hadia206

Thanks Kian.
Some comments were not addressed and also missing the user documentation.

hadia206 · 2026-05-29T21:13:14Z

+    connection = kwargs.pop("connection", None)
+    if connection:
+        return DatabaseConnection(connection)
+    required_keys = ["user", "host", "port"]


I meant ,are they optional for the user?
The docstring says they can have a default value. So should we used that as a default and not make it required for the user.
From the docstring

- host: Trino server host (str, default: "127.0.01"/"localhost") - port: Trino server port (int, default: 8080)

hadia206 · 2026-05-29T21:14:37Z

+    def convert_integer(
+        self, args: list[SQLGlotExpression], types: list[PyDoughType]
+    ) -> SQLGlotExpression:
+        # First cast to a DOUBLE, then to an INTEGER, for safety.


Let's clarify that in the comment and be specific about what is safety

hadia206 · 2026-05-29T21:14:57Z

+
+    def generate_dataframe_item_dialect_expression(
+        self, item: Any, item_type: PyDoughType
+    ) -> SQLGlotExpression:


hadia206 · 2026-05-29T21:15:23Z

+            # default version, since PyDough handles the conversion logic
+            # correctly whereas the SQLGlot version gets confused with multiple
+            # rounds of parsing and unparsing.


hadia206 · 2026-05-29T21:16:56Z

+        num_term = float(numerator.this)
+        denom_term = float(denominator.this)
+        if denom_term != 0 and num_term / denom_term == int(num_term / denom_term):
+            return exp.Literal.number(int(num_term / denom_term))


Did you verify? If yes, what's final decision?

review-notebook-app · 2026-06-01T17:34:14Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

knassre-bodo added 19 commits March 3, 2026 18:57

Initial commit setting up enum and test file

375254a

Initial dialect plumbing added

138766a

Resolving conflicts

37b1238

Added basic graphs and trino defog sql files

84c601a

Trino DOW WIP

76d9caa

WIP Trino pipeline addition

e93f57d

Up and running with mysql+postgres, need to debug 12 failing queries

63c2c60

incremental trino support increases

933769a

Debugging queries in trino WIP, need to add mongodb+cassandra

9e7fe0d

Added simplification tinkers to avoid redundant LENGTH(literal)

6642c4c

Adding division simplification tricks and updating more files based o…

e9e17c2

…n modified tests

WIP debugging various issues, adding custom datasets marker

3ac2c49

Resolving conflicts

19a0ee1

more changes WIP

4212ab2

Testing advancements and week handling WIP, some DOW bugs resolved

fd1c9e7

Testing WIP dow

9df0c0c

Mongo/cassandra adding WIP, dow/sow WIP

fb9fbca

Updated plans, down to last generation of bugs and dow/sow issues

b8b031d

Test run on CI [RUN CI][RUN TRINO]

4fda91c

knassre-bodo marked this pull request as ready for review March 30, 2026 16:10

knassre-bodo and others added 10 commits March 30, 2026 09:10

Merge branch 'main' into kian/trino

7f9324c

Test run on CI [RUN CI][RUN TRINO]

b194c25

WIP updating trino workflow

d04fdd4

Merge conflict resolved, testing CI [RUN TRINO]

3c35b34

Fixing trino workflow [RUN TRINO]

a05458f

Fixing trino workflow [RUN TRINO]

c02ee7a

WIP

63f6695

Testing fixes WIP

830065f

[pre-commit.ci] auto fixes from pre-commit.com hooks

864b099

for more information, see https://pre-commit.ci

Debugging WIP, need to fix container issues for write and several tests

67e5e1d

john-sanchez31 reviewed May 11, 2026

View reviewed changes

hadia206 reviewed May 12, 2026

View reviewed changes

knassre-bodo and others added 10 commits May 13, 2026 09:54

Abandoning trino service health check [RUN TRINO]

8450942

Try with more time [RUN TRINO]

43b0b29

Fix MySQL DB name [RUN TRINO]

aac6b26

Merge branch 'main' into kian/trino

67ffc99

Fixing mongo port [RUN TRINO]

b95ad06

Fixing cassandra port [RUN TRINO]

9e65eb3

Trino defog test revision for AVG precision

4d2f883

[RUN ALL]

6de814b

Updated non-runtime tests

45890f6

revisions

882c050

knassre-bodo requested review from hadia206 and john-sanchez31 May 26, 2026 17:00

knassre-bodo added 3 commits May 26, 2026 10:23

Adding mongo environment variables [RUN ALL]

d35b231

[RUN ALL]

018a1aa

[RUN CI][RUN TRINO]

26d140c

john-sanchez31 approved these changes May 27, 2026

View reviewed changes

knassre-bodo added 3 commits May 28, 2026 09:44

[RUN CI][RUN TRINO]

3de805e

[RUN TRINO][RUN CI]

f915e74

Fixing workflows [RUN TRINO][RUN CI]

f58c459

hadia206 reviewed May 29, 2026

View reviewed changes

knassre-bodo added 4 commits June 1, 2026 08:41

Revisions [RUN CI][RUN TRINO]

76990e2

[RUN CI][RUN TRINO]

5dcaab5

Final (hopefully) rerun [RUN CI][RUN TRINO]

3f99d8d

Added trino docs [RUN CI][RUN TRINO]

72a5a57

Final(?) run [RUN CI][RUN TRINO]

8badcff

knassre-bodo merged commit c1b25dc into main Jun 1, 2026
19 checks passed

knassre-bodo deleted the kian/trino branch June 1, 2026 19:59



		@pytest.mark.skip(
		"Trino does not properly handle all of the weird quoting cases; investigate at a later time."

	return sqlglot_expressions.Cast(this=expr, to="VARCHAR")
	return sqlglot_expressions.Cast(this=expr, to=sqlglot_expressions.DataType.build("VARCHAR"))

	self, arg: SQLGlotExpression, types: list[PyDoughType]
	self, args: list[SQLGlotExpression], types: list[PyDoughType]

	lambda self, e: f"(({self.func('DAY_OF_WEEK', e.this)} % 7) + 1)",
	lambda self, e: f"(({self.func('DAY_OF_WEEK', e.this)} % 7) + 1)"

Conversation

knassre-bodo commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

john-sanchez31 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!

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!

knassre-bodo May 13, 2026 • 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!

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!

Uh oh!

hadia206 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!

knassre-bodo May 13, 2026 • 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!

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!

knassre-bodo May 26, 2026 • 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!

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!

knassre-bodo May 13, 2026 • 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!

Uh oh!

john-sanchez31 left a comment • 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!

hadia206 left a comment

knassre-bodo commented Mar 4, 2026 •

edited

Loading

knassre-bodo May 13, 2026 •

edited

Loading

knassre-bodo May 13, 2026 •

edited

Loading

knassre-bodo May 26, 2026 •

edited

Loading

knassre-bodo May 13, 2026 •

edited

Loading

john-sanchez31 left a comment •

edited

Loading