> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 841
> > <https://reviews.apache.org/r/29900/diff/2/?file=825426#file825426line841>
> >
> >     we should add a boolean false argument at the end here, so that it does 
> > not show up in the hive-default.xml.template file.  See "hive.in.test" for 
> > example. 
> >     
> >     There are other "hive.test" params to be fixed similarly as well, we 
> > can do it as part of this one or a separate jira.

ok, will fix


> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java, line 252
> > <https://reviews.apache.org/r/29900/diff/2/?file=825431#file825431line252>
> >
> >     SessionState can be shared across multiple query executions, in 
> > hiveserver2. One case where this happens is when one user in Hue opens 
> > multiple tabs and runs queries from each of them simultaneously.
> >     
> >     This means that there can be race conditions where multiple 
> > get_timestamp invocations in single query returns different results because 
> > there was another query whose compilation started in between. (This will 
> > happen once the lock around compile is removed in HS2).
> >     
> >     We need to store this in a real query specific variable. I am still 
> > thinking what the best place for that is ..

SessionState has quite a number of query-specific fields. As there is currently 
no query-specific location for all of these to go I think it makes sense for 
this to go in SessionState for now, and it can be moved along with the rest of 
the query-specific values in SessionState if/when they get moved.


> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java, line 1404
> > <https://reviews.apache.org/r/29900/diff/2/?file=825431#file825431line1404>
> >
> >     I have seen at least another place where we have a test timestamp 
> > getting injected. I might make sense to use some kind of getTimestamp class 
> > that can be customized to give a specific timestamp. 
> >     But this does not have to be addressed in this jira.

Where does this occur?


> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java,
> >  line 31
> > <https://reviews.apache.org/r/29900/diff/2/?file=825432#file825432line31>
> >
> >     I think it would be good to clarify that for all calls within a query 
> > this returns same value. For example, if the query lifetime crosses a date 
> > boundary, you would not see two different dates for different records.
> >     
> >     Maybe reword it something like this - Returns the current date as of 
> > starting of query.

will change


> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java,
> >  line 32
> > <https://reviews.apache.org/r/29900/diff/2/?file=825432#file825432line32>
> >
> >     looks like we can consider this to be deterministic, since the value 
> > does not change within a query.

ok, will change. If we ever have a new descriptor to specify deterministic 
within the same query but different for different queries, this would fit the 
description. Would be needed for stuff like determining suitability of queries 
for materialized views.


> On Jan. 28, 2015, 8:40 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java,
> >  line 30
> > <https://reviews.apache.org/r/29900/diff/2/?file=825433#file825433line30>
> >
> >     we should update description to clarify that this is timestamp at 
> > begening of query evaluation/execution.
> >     maybe evaluation is a better word.

will change


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29900/#review70062
-----------------------------------------------------------


On Jan. 19, 2015, 10:01 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29900/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2015, 10:01 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-5472
>     https://issues.apache.org/jira/browse/HIVE-5472
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add current_date/current_timestamp. The UDFs get the current_date/timestamp 
> from the SessionState.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 25cccd7 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 0226f28 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d7c4ca7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g f412010 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g c960a6b 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java f45b20a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java
>  PRE-CREATION 
>   ql/src/test/queries/clientpositive/current_date_timestamp.q PRE-CREATION 
>   ql/src/test/results/clientpositive/current_date_timestamp.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/show_functions.q.out 9ecb0a0 
> 
> Diff: https://reviews.apache.org/r/29900/diff/
> 
> 
> Testing
> -------
> 
> qfile test added
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Reply via email to