fix(db2): update time grain expressions for DAY to use DATE function

[ysinghc]

fix(db2): use DATE() function for DAY time grain to fix sqlglot parsing error

SUMMARY

Fixes the DB2 DAY time grain expression to use the standard DATE() function instead of complex timestamp arithmetic that could not be parsed by sqlglot. This resolves chart creation failures when applying daily time grains to timestamp columns in DB2 databases.

Problem:
The previous implementation used an invalid SQL expression for the DAY time grain:

CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - MINUTE({col}) MINUTES - SECOND({col}) SECONDS - MICROSECOND({col}) MICROSECONDS

This syntax could not be parsed by sqlglot, causing ParseError exceptions and preventing users from creating charts with daily aggregations on DB2 databases.

Solution:
Changed the TimeGrain.DAY expression to use the simpler and more standard:

DATE({col})

This approach:

• Is valid DB2 SQL syntax that sqlglot can parse correctly
• Is more efficient and cleaner than the previous implementation
• Follows the same pattern used by MySQL's engine spec
• Maintains compatibility with all DB2 versions

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend fix for SQL generation

TESTING INSTRUCTIONS

1. Connect Superset to a DB2 database with a table containing timestamp columns
2. Create a chart using a timestamp column
3. Apply a "Day" time grain to the timestamp column
4. Verify the chart loads successfully without ParseError
5. Run unit tests: pytest test_db2.py -v
6. Verify all 14 tests pass, including the new test_time_grain_day_parseable test

ADDITIONAL INFORMATION

• Has associated issue: Fixes Time grain creates invalid DB2 SQL statements #35792
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

Changes:

• Modified db2.py - Updated TimeGrain.DAY expression
• Enhanced test_db2.py - Added comprehensive time grain tests including sqlglot parsing validation

Test Coverage:

• Added test_time_grain_expressions - Validates all time grain SQL expressions
• Added test_time_grain_day_parseable - Specifically tests that DAY grain produces parseable SQL
• All existing DB2 tests continue to pass

[bito-code-review]

Code Review Agent Run #1ca353

Actionable Suggestions - 0

Additional Suggestions - 4

• tests/unit_tests/db_engine_specs/test_db2.py - 4

  • Fix pytest parametrize argument type to tuple · Line 86-86
    The first argument to `pytest.mark.parametrize` should be a tuple, not a string. Change `"grain,expected_expression"` to `("grain", "expected_expression")`.
    Code suggestion

     @@ -86,1 +86,1 @@
    -    "grain,expected_expression",
    +    ("grain", "expected_expression"),

  • Replace broad exception handler with specific exception · Line 148-148
    Catching broad `Exception` is discouraged. Consider catching specific exceptions like `ParseError` or other sqlglot-specific exceptions.
    Code suggestion

     @@ -20,1 +20,2 @@
      from sqlglot import parse_one
    +from sqlglot.errors import ParseError
     @@ -148,1 +149,1 @@
    -    except Exception as e:
    +    except ParseError as e:

  • Fix docstring format to single line · Line 119-119
    The docstring should either fit on one line or follow multi-line format with summary on first line. Consider making it a single line or reformatting as multi-line.
    Code suggestion

     @@ -119,3 +119,1 @@
    -    """
    -    Test that time grain expressions generate the expected SQL.
    -    """
    +    """Test that time grain expressions generate the expected SQL."""

  • Fix multi-line docstring format and spacing · Line 129-129
    Multi-line docstring summary should start at the first line and needs a blank line between summary and description.
    Code suggestion

     @@ -129,8 +129,9 @@
    -    """
    -    Test that the DAY time grain expression generates valid SQL
    -    that can be parsed by sqlglot.
    -
    -    This test addresses the bug where the previous expression
    -    "CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ..."
    -    could not be parsed by sqlglot.
    -    """
    +    """Test that the DAY time grain expression generates valid SQL that can be parsed by sqlglot.
    +
    +    This test addresses the bug where the previous expression
    +    "CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ..."
    +    could not be parsed by sqlglot.
    +    """

Review Details

• Files reviewed - 2 · Commit Range: bb16c8a..bb16c8a

  • superset/db_engine_specs/db2.py
  • tests/unit_tests/db_engine_specs/test_db2.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[korbit-ai]

I've completed my review and didn't find any issues.

Files scanned

​

File Path	Reviewed
superset/db_engine_specs/db2.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.55%. Comparing base (76d897e) to head (d406693).
⚠️ Report is 2800 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #35848       +/-   ##
===========================================
+ Coverage   60.48%   70.55%   +10.06%     
===========================================
  Files        1931      605     -1326     
  Lines       76236    44206    -32030     
  Branches     8568     4782     -3786     
===========================================
- Hits        46114    31191    -14923     
+ Misses      28017    11759    -16258     
+ Partials     2105     1256      -849     

Flag	Coverage Δ
hive	45.70% <ø> (-3.46%)	⬇️
javascript	?
mysql	70.08% <ø> (?)
postgres	70.13% <ø> (?)
presto	49.35% <ø> (-4.45%)	⬇️
python	70.52% <ø> (+7.01%)	⬆️
sqlite	69.73% <ø> (?)
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bito-code-review]

Interaction Diagram by Bito

sequenceDiagram
participant QueryBuilder as Query Builder
participant Db2Spec as Db2EngineSpec<br/>🔄 Updated | ●●○ Medium
participant BaseSpec as BaseEngineSpec
participant TimeGrainExpr as TimeGrain Expressions<br/>🔄 Updated | ●●○ Medium
participant TestSuite as Test Suite<br/>🟩 Added | ●●● High
participant SQLParser as SQLGlot Parser
participant Database as Db2 Database
QueryBuilder->>Db2Spec: get_time_grain_expressions()
Db2Spec->>BaseSpec: get_time_grain_expressions() [inherited]
BaseSpec->>TimeGrainExpr: Copy _time_grain_expressions
TimeGrainExpr-->>BaseSpec: Return expressions dict
BaseSpec-->>Db2Spec: Return with DAY: DATE(&#123;col&#125;)
Db2Spec-->>QueryBuilder: Provide simplified DAY expression
TestSuite->>Db2Spec: Validate all time grain expressions
TestSuite->>SQLParser: Parse DAY grain SQL
SQLParser-->>TestSuite: Confirm parseability
Note over Db2Spec, TestSuite: DAY grain simplified from<br/>CAST(TIMESTAMP)... to DATE()...

Loading

Critical path: Query Builder->Db2EngineSpec->BaseEngineSpec->TimeGrain Expressions->Test Suite->SQLGlot Parser

Note: The DAY time grain expression in Db2EngineSpec was simplified from a complex CAST statement to a simpler DATE() function for better SQL parseability. Comprehensive parametrized tests were added to validate all time grain expressions and ensure the DAY expression generates valid, parseable SQL.

[bito-code-review]

Code Review Agent Run #1f63ae

Actionable Suggestions - 0

Additional Suggestions - 3

• tests/unit_tests/db_engine_specs/test_db2.py - 3

  • Multi-line docstring formatting issues · Line 120-120
    The docstring has formatting issues: it should fit on one line or start the summary on the first line. Consider reformatting as a single-line docstring.
    Code suggestion

     @@ -120,3 +120,1 @@
    -    """
    -    Test that time grain expressions generate the expected SQL.
    -    """
    +    """Test that time grain expressions generate the expected SQL."""

  • Multi-line docstring formatting and spacing · Line 130-130
    The docstring has formatting issues: summary should start on the first line and needs a blank line between summary and description. Reformat the docstring properly.
    Code suggestion

     @@ -130,7 +130,8 @@
    -    """
    -    Test that the DAY time grain expression generates valid SQL
    -    that can be parsed by sqlglot.
    -
    -    This test addresses the bug where the previous expression
    -    "CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ..."
    -    could not be parsed by sqlglot.
    -    """
    +    """Test that the DAY time grain expression generates valid SQL that can be parsed by sqlglot.
    +
    +    This test addresses the bug where the previous expression
    +    "CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ..."
    +    could not be parsed by sqlglot.
    +    """

  • Missing trailing comma in function call · Line 141-141
    Missing trailing comma after `col="my_timestamp_col"` in the function call. Add trailing comma for consistency.
    Code suggestion

     @@ -141,1 +141,1 @@
    -        col="my_timestamp_col"
    +        col="my_timestamp_col",

Review Details

• Files reviewed - 2 · Commit Range: bb16c8a..879f360

  • superset/db_engine_specs/db2.py
  • tests/unit_tests/db_engine_specs/test_db2.py
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[ysinghc]

implemented the suggestions from the bot

[sfirke]

Thanks for this! I am trying to understand why the current version is what it is, given that your version is much clearer and shorter. I traced back to the original PR that complicated things: #2545 Do you have any guess why they went from DAY({col}) to:

CAST({col} as TIMESTAMP)'
              ' - HOUR({col}) HOURS'
              ' - MINUTE({col}) MINUTES'
              ' - SECOND({col}) SECONDS'
              ' - MICROSECOND({col}) MICROSECONDS ')

? Should the change you made be made also to SECOND, MINUTE, and HOUR which look like they use similar structures? I am unfamiliar with DB2 and would love if someone could explain to me.

[ysinghc]

You are right the date({col}) function is much clearer, i assume the current version was for maximum compatibility, to work on almost any version or platform of DB2.

I can apply the modern trunc_timestamp fucntion for HOUR, MINUTE and SECONDS. It is the standard function for this operation in modern versions of db2.

[Copilot]

Pull Request Overview

This PR simplifies the DB2 TimeGrain.DAY expression from a complex CAST statement with multiple arithmetic operations to a cleaner DATE() function call, fixing a sqlglot parsing issue and improving code maintainability.

Key Changes:

• Simplified TimeGrain.DAY expression in DB2 engine spec from CAST({col} as TIMESTAMP) - HOUR({col}) HOURS - ... to DATE({col})
• Added comprehensive parametrized test coverage for all time grain expressions
• Added dedicated test to verify the DAY expression is parseable by sqlglot

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
superset/db_engine_specs/db2.py	Simplified the DAY time grain expression to use DB2's native DATE() function instead of complex timestamp arithmetic
tests/unit_tests/db_engine_specs/test_db2.py	Added comprehensive test coverage for all time grain expressions and a specific test to ensure DAY expression is sqlglot-parseable

[Copilot]

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[ysinghc]

I will try to remove these commits by the copilot and then apply any changes locally and test them before pushing them in the pr.

[Copilot]

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

[nitpick] The # noqa: S608 comment suppresses a security warning about SQL injection, but this is a false positive since expression comes from a static dictionary value, not user input. However, consider using a regular string concatenation or format string without the noqa comment, as the warning suppression is unnecessary and may raise questions during code review. Alternatively, add a comment explaining why the suppression is safe here.

[Vitor-Avila]

LGTM! I'm not the most experienced person with db2, but CC said this makes sense too, and the other syntax might have been needed years ago.

@sfirke let me know if you have any concerns here, otherwise I'll merge it

[sfirke]

LGTM!