git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Pull requests to review


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.

Julian


> 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 that
>> 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. Without
>> it
>>> - bug is still there and I could create lots of other mishit patches.
>> 
>> 
> 
> -- 
> Piotr Bojko
> http://about.me/ptr.bojko