Timestamp/Timestamptz coercion for "now - '30 days'"

[searing]

I'm having trouble expressing "now - '30 days'" in a filter expression. This short code sample illustrates what I want:

#[macro_use]
extern crate diesel;

use diesel::dsl::*;
use diesel::prelude::*;

mod schema {
    table! {
        test_table (id) {
            id -> Int8,
            foo -> Nullable<Timestamptz>,
        }
    }
}

use schema::test_table;

fn main() {
    // this is what I want to accomplish
    let foo = test_table::table
        .filter(test_table::foo.lt(diesel::dsl::sql("now() - '30 days'")));

    // this should compile
    let foo2 = test_table::table
        .filter(test_table::foo.lt((now - 30.days()).nullable()));
}

This fails to compile at "foo2" with the following error:

error[E0271]: type mismatch resolving `<diesel::expression::nullable::Nullable<diesel::expression::ops::Sub<diesel::dsl::now, diesel::expression::bound::Bound<diesel::sql_types::Interval, diesel::data_types::PgInterval>>> as diesel::Expression>::SqlType == diesel::sql_types::Nullable<diesel::sql_types::Timestamptz>`
  --> src\main.rs:25:33
   |
25 |         .filter(test_table::foo.lt((now - 30.days()).nullable()));
   |                                 ^^ expected struct `diesel::sql_types::Timestamp`, found struct `diesel::sql_types::Timestamptz`
   |
   = note: expected type `diesel::sql_types::Nullable<diesel::sql_types::Timestamp>`
              found type `diesel::sql_types::Nullable<diesel::sql_types::Timestamptz>`

Being able to get rid of the "nullable()" call would also be nice, but might be a separate issue (or difficult/impossible).

[jeremybmerrill]

@searing : Were you able to find a solution to this? I have the same problem (minus the nullable stuff), so "type mismatch resolving <diesel::expression::ops::Sub<diesel::dsl::now, diesel::expression::bound::Bound<diesel::sql_types::Interval, diesel::data_types::PgInterval>> as diesel::Expression>::SqlType == diesel::sql_types::Timestamptz" for .filter(created_at.gt( now - 1.months() ))

[searing]

No, my temporary solution is to use diesel::dsl::sql until this is fixed, hopefully in the next major Diesel release.

[jeremybmerrill]

@searing Ah okay. Thanks!

[brandur]

I'm not a maintainer, but I was looking at how this might be solved just out of curiosity. I think that the root of the problem is that the now struct is foremost fundamentally a Timestamp rather than a Timestamptz:

/// Represents the SQL `CURRENT_TIMESTAMP` constant. This is equivalent to the
/// `NOW()` function on backends that support it.
#[allow(non_camel_case_types)]
#[derive(Debug, Copy, Clone, QueryId)]
pub struct now;

impl Expression for now {
    type SqlType = Timestamp;
}

When you subtract an Interval from a Timestamp, you end up with another Timestamp, which is what's producing the error you're seeing:

impl Sub for super::Timestamp {
    type Rhs = super::Interval;
    type Output = super::Timestamp;
}

(And an off-handed comment: I found it helpful to read the obtuse error produced by the compiler backwards. When it says expected struct `diesel::sql_types::Timestamp`, found struct `diesel::sql_types::Timestamptz` , what it really means is that it wanted a Timestamptz based off your schema definition, and found Timestamp.)

Coerce is implemented for now so that it can become a Timestamptz:

#[cfg(feature = "postgres")]
impl AsExpression<Timestamptz> for now {
    type Expression = Coerce<now, Timestamptz>;

    fn as_expression(self) -> Self::Expression {
        Coerce::new(self)
    }
}

#[cfg(feature = "postgres")]
impl AsExpression<Nullable<Timestamptz>> for now {
    type Expression = Coerce<now, Nullable<Timestamptz>>;

    fn as_expression(self) -> Self::Expression {
        Coerce::new(self)
    }
}

But it doesn't seem to happen soon enough. I would think that even after the interval subtraction, it should be able to coerce the resulting Timestamp to a Timestamptz, but it apparently doesn't, and even after reading the code for some time I still don't understand how exactly coerce is supposed to work. There's a little documentation explaining what it's for, but neither it nor its implementation provide much in the way of hints as to what it's doing.

I was able to fix the problem by introducing a new struct nowtz for Postgres (and appropriate add/sub implementations):

diff --git a/diesel/src/expression/functions/date_and_time.rs b/diesel/src/expression/functions/date_and_time.rs
index 1461b8e7..d931bce9 100644
--- a/diesel/src/expression/functions/date_and_time.rs
+++ b/diesel/src/expression/functions/date_and_time.rs
@@ -46,6 +46,35 @@ let today: chrono::NaiveDate = diesel::select(date(now)).first(&connection).unwr
 ",
 "The return type of [`date(expr)`](../dsl/fn.date.html)");
 
+#[allow(non_camel_case_types)]
+#[cfg(feature = "postgres")]
+#[derive(Debug, Copy, Clone, QueryId)]
+pub struct nowtz;
+
+#[cfg(feature = "postgres")]
+impl Expression for nowtz {
+    type SqlType = Timestamptz;
+}
+
+#[cfg(feature = "postgres")]
+impl NonAggregate for nowtz {}
+
+#[cfg(feature = "postgres")]
+impl<DB: Backend> QueryFragment<DB> for nowtz {
+    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+        out.push_sql("CURRENT_TIMESTAMP");
+        Ok(())
+    }
+}
+
+#[cfg(feature = "postgres")]
+impl_selectable_expression!(nowtz);
+
+#[cfg(feature = "postgres")]
+operator_allowed!(nowtz, Add, add);
+#[cfg(feature = "postgres")]
+operator_allowed!(nowtz, Sub, sub);
+
 #[cfg(feature = "postgres")]
 use expression::AsExpression;
 #[cfg(feature = "postgres")]
diff --git a/diesel/src/sql_types/ops.rs b/diesel/src/sql_types/ops.rs
index afb35d67..46f53678 100644
--- a/diesel/src/sql_types/ops.rs
+++ b/diesel/src/sql_types/ops.rs
@@ -170,3 +170,27 @@ impl Sub for super::Nullable<super::Timestamp> {
     type Rhs = super::Nullable<super::Interval>;
     type Output = super::Nullable<super::Timestamp>;
 }
+
+#[cfg(feature = "postgres")]
+impl Add for super::Timestamptz {
+    type Rhs = super::Interval;
+    type Output = super::Timestamptz;
+}
+
+#[cfg(feature = "postgres")]
+impl Add for super::Nullable<super::Timestamptz> {
+    type Rhs = super::Nullable<super::Interval>;
+    type Output = super::Nullable<super::Timestamptz>;
+}
+
+#[cfg(feature = "postgres")]
+impl Sub for super::Timestamptz {
+    type Rhs = super::Interval;
+    type Output = super::Timestamptz;
+}
+
+#[cfg(feature = "postgres")]
+impl Sub for super::Nullable<super::Timestamptz> {
+    type Rhs = super::Nullable<super::Interval>;
+    type Output = super::Nullable<super::Timestamptz>;
+}

And then changing your line above to use nowtz instead of now:

    let foo2 = test_table::table
        .filter(test_table::foo.lt((nowtz - 30.days()).nullable()))

... but I assume that there's a much more elegant way to go about this which is a little beyond my grasp. If anyone more familiar with the type system could give me a high level explanation for how this might be generally solved, I could try my hand at sending over a patch.

[searing]

I suppose you could introduce a new SQL type (perhaps Now) which can be coerced to either Timestamp or Timestamptz, and then implement interval addition/subtraction for that type. That seems like the most "correct" way to do it, in the sense that the Postgres server is probably doing something vaguely similar. I'm not sure how we'd feel about adding a new SQL type for what might be a fairly simple case, but I think it's better than nowtz since, to the user, the extra Now type should be totally transparent.

[brandur]

I suppose you could introduce a new SQL type (perhaps Now) which can be coerced to either Timestamp or Timestamptz, and then implement interval addition/subtraction for that type.

So the idea there would be that Now - Interval = Now so that you could then safely coerce to either Timestamp or Timestamptz right? I couldn't say for sure whether that would work, but it sounds like a reasonable idea. The way Diesel seems modeled today though is that SQL types are modeled after actual types in SQL, so the abstraction might still be a bit leaky.

That seems like the most "correct" way to do it, in the sense that the Postgres server is probably doing something vaguely similar.

I took a quick look at the datetime docs in Postgres and interestingly, now() is stated to unequivocally return a timestamp with time zone. It must be coerced back to a regular timestamp as needed.

I'm not sure how we'd feel about adding a new SQL type for what might be a fairly simple case, but I think it's better than nowtz since, to the user, the extra Now type should be totally transparent.

Yeah, I tried my current solution to verify that I had at least a partial understanding of the problem, but it's not very good.