Could this move out of the preimage impl?

it could, I was just following the style for the actual invocation.

Does your comment about this still hold?

SELECT  PULocationID
           ,pickup_datetime
      FROM taxi_view_2025
     WHERE date_trunc('month', pickup_datetime) = '2025-12-03'

Without preimage it would always return False, but with preimage, we create an interval [2025-12-01, 2026-01-01) and simplification rule returns col >= 2025-12-01 and col < 2026-01-01 we could get false positives, because 2025-12-03 falls into that interval.

We'd need to change the behavior to cover this.

• One way would be by having a guard checking the eq Operator specifically for date_trunc preimage and return PreimageResult::None if rhs != date_trunc(granularity, rhs). But preimage is Operator agnostic.
• Another way is by having an optimization rule to do check this rhs != date_trunc(granularity, rhs) and return False for the whole column. But that's adding a rule just for one udf.
• Another way is to only let date_trunc preimage work with with rhs = date_trunc(granularity, rhs), but this requires the user to write the date in the right way if they want the query to run faster.
  For example:

++WHERE date_trunc('month', pickup_datetime) = '2025-12-01'
--WHERE date_trunc('month', pickup_datetime) = '2025-12-03'

It was actually covered for floor preimage impl by @devanshu0987 in #20059
Check here:
https://github.com/apache/datafusion/pull/20059/changes#diff-077176fcf22cb36a0a51631a43739f5f015f46305be4f49142a450e25b152b84R280-R303
Floor is very similar to date_trunc, so we could replicate the behavior.

I don't understand why None is returned in the case that a clear value is known. For = '2025-12-03', the value should be False. I assumed that None basically means that the preimage could not be determined because something was invalid (an error). If you use None for valid cases, how do you distinguish invalid cases?

I guess I have a clarifying question:

What should the interval be in non-obvious cases? What happens if the Interval is None (it seems rewrite_with_preimage is only called on an actual Interval)?

Also, looking at it a bit more, I think rewrite_with_preimage is wrong in some cases.

Here are the notes I have in our implementation:

        // Special condition for these operators:
        // if date_trunc(part, const_rhs) == const_rhs,
        // then we use `const_rhs` instead of `next_interval(part, const_rhs)`,
        // lhs(<)  --> column <  next_interval(part, const_rhs)
        // lhs(>=) --> column >= next_interval(part, const_rhs)

In summary, rewrite_with_preimage should compare with the upper instead of the lower for truncation.

date_trunc('month', pickup_datetime) < '2025-12-03' would become
pickup_datetime < '2026-01-01' because this should be true even if
pickup_datetime == '2025-12-31' because date_trunc('month', '2025-12-31') == '2025-12-01'

For = '2025-12-03', the value should be False.

But if we use preimage it will simplify the expression to col >= 2025-12-01 and col < 2026-01-01 with month granularity.

This way column values can match the expression and return True even if they are not exactly 2025-12-03. This gives us false positives on = comparisons when the rhs is not itself truncated to the same granularity (i.e., round-trip doesn't hold - date_trunc(granularity, rhs) != rhs).

I propose following the Floor example and only use preimage for date_trunc when date_trunc(granularity, rhs) = rhs

If there is no valid interval, there is no preimage

(but, from those notes I also realized I was handling the interval wrong in some cases)

But these are the notes relevant to this specific case (predicate operator is =):

// Special condition:
// if date_trunc(part, const_rhs) != const_rhs, this is always false
// For this operator, truncation means that we check if the column is INSIDE of a range.
// lhs(=) --> column >= date_trunc(part, const_rhs) AND column < next_interval(part, const_rhs)

date_trunc('month', pickup_datetime) = '2025-12-03' is always false.
date_trunc('month', pickup_datetime) < '2025-12-03' requires an interval to be returned.

Without knowing if the predicate operator is = or <, preimage cannot know whether to return an Interval or None (if None is even the correct return in that case). So preimage must return the interval. But, in rewrite_with_preimage, you can do the appropriate check:

Operator::Eq && lower != <original> => False,
Operator::Eq => and(<check if within interval>),

I'm not sure if this makes sense for intervals from non-truncating functions. I'd have to simmer on that...

We should add a guard for Time types granularities.
We could reuse this function.

We should add a guard for Type families. col_expr: TimeStamp needs a lit_expr: TimeStamp and the same for Time types.

as in if col_expr is a TimeStamp type, then lit_expr must also be a TimeStamp type? Why is that the case?

If I have a nanosecond timestamp (time since epoch) and the comparison is a Time type, if I convert both to nanosecond timestamps aren't they still comparable?

Actually, shouldn't this type of validation be upstream of preimage in whatever function decomposes the predicate?

general_date_trunc converts the values back to the original TimeStamp type, we shouldn't increment in nanos, but use the original TimeUnit.

return Err or None?

I can update the comment, probably just copy pasta.

This is same behavior as other increment functions (which only increment the correct time type at appropriate granularities)

Keep match arms for granularities finer than rhs?

this is for incrementing milliseconds. If you increment by 1 when the granularity is microseconds then you've incremented by too much. If you have a timestamp in milliseconds and you're truncating microseconds, you should have no change because your timestamp is too coarse.