Support DATETIME function in PyDough #267

knassre-bodo · 2025-02-14T21:34:33Z

Resolves #208. See issue for more details. Full list of additional changes:

Ensured the DATETIME operator works for both sqlite & ANSI with very permissive spacing/capitalization/alias rules for the various string literals.
- now (& similar aliases): for ANSI generates CURRENT_TIMESTAMP(), for sqlite generates DATETIME('now')
- start of <unit> (year/month/day): for ANSI generates DATE_TRUNC(<unit>, dt), for sqlite generates DATETIME(dt, 'start of <unit>')
- start of <unit> (hour/minute/second): for ANSI generates DATE_TRUNC(<unit>, dt), for sqlite generates STRFTIME(fmt_str, dt) where the fmt_str truncates to that unit.
- ±<amt> <unit>: for ANSI generates DATEADD(<unit>, ±<amt>dt), for sqlite generates DATETIME(dt, '±<amt> <unit>)
Moves a lot of the datetime unit logic in the transform bindings into a DateTimeUnit enum to handle common logic, such as converting a unit alias into a cannonical form or identifying the format string used to truncate to that unit.
Added new fixture to create a DatabaseContext without a connection for each supported dialect
Changed the tests in test_pydough_to_sql.py to be parameterized with the afformentioned new fixture so sqlite & ANSI are both tested
Changed the refsol .sql files to contain the dialect (e.g. xxx_ansi.sql vs xxx_sqlite.sql)
Fixed miscellaneous typos found in the repo
Adjusted the SQLGlot generation of the empty singleton to avoid a malformed VALUES clause

…nversion

…ts and fill out comments/docstrings

knassre-bodo · 2025-02-17T19:09:43Z

tests/test_sql_refsols/simple_values_ansi.sql

 FROM (VALUES
-  ())
+  (NULL))


Just having VALUES () is invalid, so using VALUES (NULL) helps

…rated

knassre-bodo · 2025-02-17T19:28:14Z

tests/test_sql_refsols/hour_minute_day_sqlite.sql

@@ -0,0 +1,47 @@
+SELECT


These are now automatically generated along with the XXX_ansi.sql file for all the tests in test_pydough_to_sql.py, so we can verify that each supported dialect works on the expected functions.

knassre-bodo · 2025-02-17T19:28:56Z

tests/conftest.py

@@ -153,14 +154,14 @@ def impl(file_name: str) -> str:


 @pytest.fixture(scope="session")
-def get_sql_test_filename() -> Callable[[str], str]:
+def get_sql_test_filename() -> Callable[[str, DatabaseDialect], str]:


This change allows the SQL files to be dialect-specific.

knassre-bodo · 2025-02-17T19:29:29Z

tests/conftest.py

+        pytest.param(DatabaseDialect.SQLITE, id="sqlite"),
+    ]
+)
+def empty_context_database(request) -> DatabaseContext:


This creates a new fixture with a DatabaseContext w/o an actual connection for every dialect, so we can test SQL generation for all dialects.

knassre-bodo · 2025-02-17T19:30:22Z

tests/simple_pydough_functions.py

+    )
+
+
+def datetime_sampler():


This doesn't cover every edge case for the string parsing of DATETIME, but covers a large number of them that were generated to account for various units, aliases, signs, spacings, capitalizations, etc.

knassre-bodo · 2025-02-17T19:31:12Z

pydough/sqlglot/transform_bindings.py

+"""
+
+
+class DateTimeUnit(Enum):


Moved logic into this enum so that DATETIME wouldn't duplicate its unit-handling logic with DATEDIFF.

knassre-bodo · 2025-02-17T19:40:48Z

pydough/metadata/collections/collection_metadata.py

-    # Set of names of of fields that can be included in the JSON
+    # Set of names of fields that can be included in the JSON


This was a typo I mass-fixed

knassre-bodo · 2025-02-17T19:42:29Z

pydough/sqlglot/transform_bindings.py

+                if isinstance(base, sqlglot_expressions.Datetime):
+                    base.this.append(trunc_expr)
+                    return base


This edge case allows us to flatten DATETIME(DATETIME('now', '-1 day'), 'start of year') into DATETIME('now', '-1 day', 'start of year').

Clarity purposes: Does the visitor revisit after appending? Maybe we can keep a small comment for further programmers?

The visitor doesn't visit SQLGlot nodes, it returns them. In this case, base is a SQLGlot expression node returned earlier by the SQLGlot relational expression visitor (which created base from a relational expression). All we're doing is modifying base after the fact, instead of returning a new object containing it.

knassre-bodo · 2025-02-17T19:42:49Z

pydough/sqlglot/transform_bindings.py

+        if isinstance(base, sqlglot_expressions.Datetime):
+            base.this.append(offset_expr)
+            return base


This edge case allows us to flatten DATETIME(DATETIME('now', 'start of year'), '-1 day') into DATETIME('now', 'start of year', '-1 day').

vineetg3 · 2025-02-18T17:15:18Z

pydough/sqlglot/transform_bindings.py

+    Returns:
+        The SQLGlot expression to truncate `base`.
+    """
+    if dialect == DatabaseDialect.SQLITE:


For consistency with other SQLite bindings, we should either create a new function here or refactor the other bindings to align with the current conditional approach.

The problem is that the core behavior is the same, but the helper function changes. What I probably want, in future, is to overhaul how the bindings work so there is a measure of inheritance. That way, there can be general superclasses that deal with most of the behavior, and subclasses can override specific helpers, and there can be inheritance as fallbacks. Whether this is literally via classes, or just a stack of objects, needs to be designed.

In other words, I agree with your sentiment, but I want fixing this to be part of a larger overhaul/refactor of the file.

pydough/sqlglot/transform_bindings.py

vineetg3 · 2025-02-18T18:24:29Z

tests/simple_pydough_functions.py

+            " \n \t312 \nD \n\r ",
+        ),
+        DATETIME(" Current_Date ", "+ 45 MM", "-135 s"),
+    )


Suggestion: Might be far-fetched but how about adding tests for camel-cases such as DATETIME("cUrReNt_Timestamp").

🤷 might as well

Just added a few weird ones.

tests/test_pydough_to_sql.py

knassre-bodo added 9 commits February 14, 2025 16:33

Added operator and simple tests for DATETIME function, need to add co…

d16ab54

…nversion

Started sqlite impl, and fixed some weird grammar errors

1a7eced

Resolving conflicts

e8cbc34

Fully implemented datetime & have tests working, need to add ANSI tes…

99db5ac

…ts and fill out comments/docstrings

Adding/modifying SQL tests

744c7fa

Adding docs and updating tests

7b91264

Refactoring to use enums instead of booleans/strings

0dff920

Adding more exhaustive SQL test

31b3186

Switching sql tests to be dialect-specific

95b3d4f

knassre-bodo commented Feb 17, 2025

View reviewed changes

knassre-bodo added 2 commits February 17, 2025 14:17

Fixing ANSI generation of DATE_TRUNC and making sampler test non-gene…

178b288

…rated

Adding sqlite parameterization for all to_sql tests

902df5e

knassre-bodo commented Feb 17, 2025

View reviewed changes

Compacting sqlite terms and moving mroe logic into enum

2e239bd

knassre-bodo commented Feb 17, 2025

View reviewed changes

[RUN CI]

2792c0e

knassre-bodo marked this pull request as ready for review February 17, 2025 19:49

knassre-bodo requested a review from vineetg3 February 17, 2025 19:49

vineetg3 reviewed Feb 18, 2025

View reviewed changes

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

vineetg3 reviewed Feb 18, 2025

View reviewed changes

tests/test_pydough_to_sql.py Outdated Show resolved Hide resolved

Renaming ansi tests to sqlite when they should say sqlite [RUN CI]

9b655b3

knassre-bodo requested a review from vineetg3 February 18, 2025 20:57

Resolving conflicts [RUN CI]

cad1bc8

knassre-bodo merged commit 5fc012c into main Feb 19, 2025
5 checks passed

knassre-bodo deleted the kian/datetime branch February 19, 2025 03:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support DATETIME function in PyDough #267

Support DATETIME function in PyDough #267

knassre-bodo commented Feb 14, 2025 •

edited

Loading

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025 •

edited

Loading

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

vineetg3 Feb 18, 2025

knassre-bodo Feb 18, 2025

knassre-bodo Feb 17, 2025

vineetg3 Feb 18, 2025

knassre-bodo Feb 18, 2025 •

edited

Loading

knassre-bodo Feb 18, 2025

vineetg3 Feb 18, 2025

knassre-bodo Feb 18, 2025 •

edited

Loading

knassre-bodo Feb 18, 2025

		# Set of names of of fields that can be included in the JSON
		# Set of names of fields that can be included in the JSON

Support DATETIME function in PyDough #267

Support DATETIME function in PyDough #267

Conversation

knassre-bodo commented Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo commented Feb 14, 2025 •

edited

Loading

knassre-bodo Feb 17, 2025 •

edited

Loading

knassre-bodo Feb 18, 2025 •

edited

Loading

knassre-bodo Feb 18, 2025 •

edited

Loading