[ 
https://issues.apache.org/jira/browse/PHOENIX-5066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17437183#comment-17437183
 ] 

Istvan Toth commented on PHOENIX-5066:
--------------------------------------

{quote}Is it as simple as "old style" and whatever you think the most correct 
"new style" is for timezone handling? As long as we have the "socket" to push 
this information everywhere it needs to go, we have the flexibility to let 
users tell us what it should be (either through JDBC url or Properties).
{quote}
Yes, that's the idea.
{quote}However, if DDL operations need it too, we'd maybe have to update the 
proto requests on MetaDataEndpointImpl?
{quote}
My current plan does not manifest the option in the DB schema. Do you think 
that handling this on a table or column basis is something worth exploring ?
{quote}Yeah, I don't think you would _need_ a ThreadLocal – the scanner 
(handler) should all be done within one thread.
{quote}
We can push the context to the RS via a scan attribute, but we still need to 
push the same information to the expression evaluation code somehow.
We do not NEED ThreadLocal as such, it is a way to simplify the code and 
optimize out pushing the time context info through every single evaluate() call.

Say we have

{noformat}
select left(to_string(birthday),4)-1900 from birthday where 
person='joe';{noformat}

In this case, we need the context to evaluate _to_string(birthday)_.
However, this is several levels down in the expression tree, and we need some 
way to push this information there.
AFAICT the expression code follows a strict functional model, so we'd have to 
add the TZ/format information (or some kind of context) to the _evaluate()_ 
method of _to_string_, _left_, and _multiply_ expressions (effectively to ALL 
expressions), and to any methods that may call that, and do not hold a 
reference to the TZ/format data for current query or scan.

The ThreadLocal would be used to provide a global constant for the thread, 
since we cannot use a JVM level singleton or static class due to the 
restrictions above. (i.e, two clients from different timezones running a scan 
which evalutes the above expression in the same RS, or similar issues on the 
client side)

AFAICT the choice is:
1. Modify the Expression interface to add a context parameter to evaluate(), 
and push it down to all children even if not used.
2. Use a ThreadLocal that we set once (when creating the Connection object on 
the client side, and when starting the scan on the server side), and get the 
same context from there.

I am not completely happy with either one :(

> The TimeZone is incorrectly used during writing or reading data
> ---------------------------------------------------------------
>
>                 Key: PHOENIX-5066
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-5066
>             Project: Phoenix
>          Issue Type: Bug
>    Affects Versions: 5.0.0, 4.14.1
>            Reporter: Jaanai Zhang
>            Assignee: Istvan Toth
>            Priority: Critical
>             Fix For: 4.17.0, 5.2.0, 4.16.2
>
>         Attachments: DateTest.java, PHOENIX-5066.4x.v1.patch, 
> PHOENIX-5066.4x.v2.patch, PHOENIX-5066.4x.v3.patch, 
> PHOENIX-5066.master.v1.patch, PHOENIX-5066.master.v2.patch, 
> PHOENIX-5066.master.v3.patch, PHOENIX-5066.master.v4.patch, 
> PHOENIX-5066.master.v5.patch, PHOENIX-5066.master.v6.patch
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> We have two methods to write data when uses JDBC API.
> #1. Uses _the exceuteUpdate_ method to execute a string that is an upsert SQL.
> #2. Uses the _prepareStatement_ method to set some objects and execute.
> The _string_ data needs to convert to a new object by the schema information 
> of tables. we'll use some date formatters to convert string data to object 
> for Date/Time/Timestamp types when writes data and the formatters are used 
> when reads data as well.
>  
> *Uses default timezone test*
>  Writing 3 records by the different ways.
> {code:java}
> UPSERT INTO date_test VALUES (1,'2018-12-10 15:40:47','2018-12-10 
> 15:40:47','2018-12-10 15:40:47') 
> UPSERT INTO date_test VALUES (2,to_date('2018-12-10 
> 15:40:47'),to_time('2018-12-10 15:40:47'),to_timestamp('2018-12-10 15:40:47'))
> stmt.setInt(1, 3);stmt.setDate(2, date);stmt.setTime(3, 
> time);stmt.setTimestamp(4, ts);
> {code}
> Reading the table by the getObject(getDate/getTime/getTimestamp) methods.
> {code:java}
> 1 | 2018-12-10 | 23:45:07 | 2018-12-10 23:45:07.0 
> 2 | 2018-12-10 | 23:45:07 | 2018-12-10 23:45:07.0 
> 3 | 2018-12-10 | 15:45:07 | 2018-12-10 15:45:07.66 
> {code}
> Reading the table by the getString methods 
> {code:java}
> 1 | 2018-12-10 15:45:07.000 | 2018-12-10 15:45:07.000 | 2018-12-10 
> 15:45:07.000 
> 2 | 2018-12-10 15:45:07.000 | 2018-12-10 15:45:07.000 | 2018-12-10 
> 15:45:07.000 
> 3 | 2018-12-10 07:45:07.660 | 2018-12-10 07:45:07.660 | 2018-12-10 
> 07:45:07.660
> {code}
>  *Uses GMT+8 test*
>  Writing 3 records by the different ways.
> {code:java}
> UPSERT INTO date_test VALUES (1,'2018-12-10 15:40:47','2018-12-10 
> 15:40:47','2018-12-10 15:40:47')
> UPSERT INTO date_test VALUES (2,to_date('2018-12-10 
> 15:40:47'),to_time('2018-12-10 15:40:47'),to_timestamp('2018-12-10 15:40:47'))
> stmt.setInt(1, 3);stmt.setDate(2, date);stmt.setTime(3, 
> time);stmt.setTimestamp(4, ts);
> {code}
> Reading the table by the getObject(getDate/getTime/getTimestamp) methods.
> {code:java}
> 1 | 2018-12-10 | 23:40:47 | 2018-12-10 23:40:47.0 
> 2 | 2018-12-10 | 15:40:47 | 2018-12-10 15:40:47.0 
> 3 | 2018-12-10 | 15:40:47 | 2018-12-10 15:40:47.106 {code}
> Reading the table by the getString methods
> {code:java}
>  1 | 2018-12-10 23:40:47.000 | 2018-12-10 23:40:47.000 | 2018-12-10 
> 23:40:47.000
> 2 | 2018-12-10 15:40:47.000 | 2018-12-10 15:40:47.000 | 2018-12-10 
> 15:40:47.000
> 3 | 2018-12-10 15:40:47.106 | 2018-12-10 15:40:47.106 | 2018-12-10 
> 15:40:47.106
> {code}
>  
> _We_ have a historical problem,  we'll parse the string to 
> Date/Time/Timestamp objects with timezone in #1, which means the actual data 
> is going to be changed when stored in HBase table。



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to