Re: [PR] fix(pinot): dialect date truncation [superset]

Fri, 17 Oct 2025 22:08:40 -0700 


korbit-ai[bot] commented on code in PR #35420:
URL: https://github.com/apache/superset/pull/35420#discussion_r2395136243


##########
superset/sql/dialects/pinot.py:
##########
@@ -95,3 +101,8 @@ def datatype_sql(self, expression: exp.DataType) -> str:
                 return f"{type_sql} UNSIGNED{nested}"
 
             return f"{type_sql}{nested}"
+
+        def cast_sql(self, expression: exp.Cast, safe_prefix: str | None = 
None) -> str:
+            # Pinot doesn't support MySQL's TIMESTAMP() function
+            # Use standard CAST syntax instead
+            return super(MySQL.Generator, self).cast_sql(expression, 
safe_prefix)

Review Comment:
   ### Incorrect super() call bypasses parent class <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The super() call bypasses the immediate parent class MySQL.Generator and 
calls the grandparent class directly, which may not provide the expected cast 
functionality for Pinot.
   
   
   ###### Why this matters
   This could result in incorrect SQL generation if the grandparent class has 
different casting behavior than what Pinot expects, potentially causing runtime 
errors or unexpected query results.
   
   ###### Suggested change ∙ *Feature Preview*
   Use the standard super() call to properly invoke the parent class method:
   ```python
   def cast_sql(self, expression: exp.Cast, safe_prefix: str | None = None) -> 
str:
       # Pinot doesn't support MySQL's TIMESTAMP() function
       # Use standard CAST syntax instead
       return super().cast_sql(expression, safe_prefix)
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ccfcfc31-d202-4032-844b-d453744d3905/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ccfcfc31-d202-4032-844b-d453744d3905?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ccfcfc31-d202-4032-844b-d453744d3905?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ccfcfc31-d202-4032-844b-d453744d3905?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ccfcfc31-d202-4032-844b-d453744d3905)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:3dbb13b4-22a4-4bb3-8cc7-3d733e607df2 -->
   
   
   [](3dbb13b4-22a4-4bb3-8cc7-3d733e607df2)



##########
superset/sql/dialects/pinot.py:
##########
@@ -78,6 +78,12 @@ class Generator(MySQL.Generator):
             exp.DataType.Type.UBIGINT: "UNSIGNED",
         }
 
+        TRANSFORMS = {
+            **MySQL.Generator.TRANSFORMS,
+        }
+        # Remove DATE_TRUNC transformation - Pinot supports standard SQL 
DATE_TRUNC
+        TRANSFORMS.pop(exp.DateTrunc, None)

Review Comment:
   ### Inefficient dictionary copy and modify pattern <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Dictionary is copied and then immediately modified, creating unnecessary 
overhead during class initialization.
   
   
   ###### Why this matters
   This pattern creates a full copy of the parent's TRANSFORMS dictionary only 
to remove one key, which wastes memory and CPU cycles during every class 
instantiation.
   
   ###### Suggested change ∙ *Feature Preview*
   Use dictionary comprehension or dict() constructor with filtering to create 
the modified dictionary in one operation:
   
   ```python
   TRANSFORMS = {
       k: v for k, v in MySQL.Generator.TRANSFORMS.items() 
       if k != exp.DateTrunc
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c15b4dc3-0ed7-45d3-9908-ea25f2daaed2/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c15b4dc3-0ed7-45d3-9908-ea25f2daaed2?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c15b4dc3-0ed7-45d3-9908-ea25f2daaed2?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c15b4dc3-0ed7-45d3-9908-ea25f2daaed2?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c15b4dc3-0ed7-45d3-9908-ea25f2daaed2)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:6b49b504-da1f-4a0a-8dc6-d1cf42d3f580 -->
   
   
   [](6b49b504-da1f-4a0a-8dc6-d1cf42d3f580)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to