Re: Pull requests to review
Correct me if I am wrong.
For test UDFTest.testDate line ~823 conversion from TimestampString
(TIMESTAMP '1970-01-01 00:00:00') is done via a call to
RexLiteral.getValueAs(Long.class). There is no explicit time zone here
provided, only the implicit one based on
TimestampString.getMillisSinceEpoch method definition.
Should the table functions implementation for timestamp parameters support
the same thinking as for other UDFs?
On Tue, Nov 6, 2018 at 12:44 AM Julian Hyde <jhyde@xxxxxxxxxx> wrote:
> A TimestampString has no timezone. It does not represent a moment in time.
> It represents a position of the hands on a clock.
> A Timestamp does not have a timezone either, but it represents a moment in
> time. (Internally represented by milliseconds since the UTC epoch.)
> Therefore, to go from a TimestampString to a Timestamp, or vice versa, you
> need a time zone. I’m not sure where that should come from in your code.
> > On Nov 5, 2018, at 1:19 PM, ptr.bojko@xxxxxxxxx wrote:
> > Julian, so.. is it correct to translate TimestampString to
> > java.sql.Timestamp on RexToLixTranslator.convert(...) level as in pull
> > request https://github.com/apache/calcite/pull/900/commits ?
> > I hope so, then I could revoke pull request
> > https://github.com/apache/calcite/pull/878
> > On Mon, Nov 5, 2018 at 9:34 PM Julian Hyde <jhyde@xxxxxxxxxx> wrote:
> >> Yeah, TimestampString is for SqlNode and RexNode only. Not JDBC code.
> >> Likewise DateString, TimeString, NlsString. Sorry the doc didn’t make
> >> clear. Although frankly it’s impractical to document all of the places
> >> something is NOT used.
> >>> On Nov 5, 2018, at 5:49 AM, ptr.bojko@xxxxxxxxx wrote:
> >>> Vladimir,
> >>> I thought that TimestampString IS the appropriate type :D (knowledge
> >> based
> >>> on Rexbuilder code, where TimestampString is being created as
> >> RexLiteral).
> >>> My problem is that I don't see what is the scope of TimestampString,
> >>> DateString, etc in Calcite. Does it span to Rel/Rex tree? Or it should
> >> not?
> >>> This is why I've created two different patches :(
> >>> Any help with the responsibility of TimestampString appreciated.
> >> it
> >>> - bug is still there and I could create lots of other mishit patches.
> > --
> > Piotr Bojko
> > http://about.me/ptr.bojko