I've recorded my thoughts on this in the attached document
A few notes:
- TimestampAdd has very well-spec'd behavior as part of ODBC. For example
the semantics for how to handle adding a second to a date value are clearly
defined. I experimented with a few different databases for how this is
handled with datetime_plus and didn't find it consistent.
- Transforming a TimestampAdd expression to datetime_plus would require
multiplying a 'unit' interval by the expression used for the interval
parameter in TimestampAdd. This looks odd in generated SQL, and if you were
go to back to timestampadd, the original intent wouldn't be clear.
- There are a few units in timestampadd that are not directly supported by
intervals (week, quarter, millennium, etc)
So I wouldn't recommend datetime_plus. I'd recommend a new structure
similar to timestampadd, but with features to transform to datetime_plus.
On Mon, May 28, 2018 at 6:52 PM, Julian Hyde <jh...@apache.org> wrote:
> I think we should use the datetime_plus operator. It is standard, and
> sufficiently general. Its second argument needs to be an interval
> value, not necessarily an interval literal.
>
> On Mon, May 28, 2018 at 2:35 PM, James Duong <jdu...@dremio.com> wrote:
> > There are essentially two ways to add an interval to a datetime vaue in
> > Calcite
> >
> > Call the timestampadd() function:
> > select {fn timestampadd(year, 1, hire_date)}...
> >
> > Use datetime_plus interval arithmetic:
> > select hire_date + interval '1' year
> >
> >
> > Note that timestampadd's second argument does not need to be a literal.
> > Often it is a column expression. For datetime_plus literals are usually
> > used.
> >
> > I propose we create a new SqlOperator that can canonicalize both of these
> > inputs into one node. This lets us apply any transformations on this
> > canonical type regardless of what the original query was.
> >
> > It takes in the following arguments:
> > 1. A date/time/timestamp input
> > 2. an interval input as an integer
> > 3. a time unit for the input
> > 4. a synthetic argument indicating the source form of the function call
> > (either datetime_plus or timestampadd).
> >
> >
> > The idea is that this canonical form is easy to get into for both types,
> > and provides methods to easily convert to either type. This would help
> with
> > unparsing in SqlDialects (you do not need to implement pushdown for both
> > types of inputs).
>
