alamb commented on code in PR #22837:
URL: https://github.com/apache/datafusion/pull/22837#discussion_r3390175108


##########
datafusion/optimizer/tests/optimizer_integration.rs:
##########
@@ -795,7 +795,7 @@ fn extension_node_does_not_block_projection_pruning() -> 
Result<()> {
         Projection: t.a, CAST(t.ts AS Timestamp(ms, "UTC")) AS ts
           Filter: __common_expr_3 > TimestampMillisecond(1000, Some("UTC")) 
AND __common_expr_3 < TimestampMillisecond(2000, Some("UTC"))
             Projection: CAST(t.ts AS Timestamp(ms, "UTC")) AS __common_expr_3, 
t.a, t.ts
-              TableScan: t projection=[a, ts], partial_filters=[t.ts > 
TimestampNanosecond(1000000000, None), t.ts < TimestampNanosecond(2000000000, 
None), CAST(t.ts AS Timestamp(ms, "UTC")) > TimestampMillisecond(1000, 
Some("UTC")), CAST(t.ts AS Timestamp(ms, "UTC")) < TimestampMillisecond(2000, 
Some("UTC"))]
+              TableScan: t projection=[a, ts], partial_filters=[CAST(t.ts AS 
Timestamp(ms, "UTC")) > TimestampMillisecond(1000, Some("UTC")), CAST(t.ts AS 
Timestamp(ms, "UTC")) < TimestampMillisecond(2000, Some("UTC"))]

Review Comment:
   I think tje inequality still holds for all possible ns values of `t.ts`, so 
this seems like a significantly worse plan as now the column must be cast 
rather than comparing directly to a constant 🤔 
   
   Specifically I think this predicate
   
   ```sql
   CAST(t.ts AS Timestamp(ms, "UTC")) > TimestampMillisecond(1000, Some("UTC"))
   ```
   
   Evaluates to `true` for the exact same values of `t.ts` (but is more 
efficient to implement) than this predicate:
   
   
   ```sql
   t.ts > TimestampNanosecond(1000000000, None)
   ```
   
   I do see that this doesn't hold for equality
   
   ```sql
   CAST(t.ts AS Timestamp(ms, "UTC")) = TimestampMillisecond(1000, Some("UTC"))
   ```
   
   is NOT the same as 
   
   ```sql
   t.ts = TimestampNanosecond(1000000000, None)
   ```
   
   A counter example being `1000000001`



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