You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "John Russell (Code Review)" <ge...@cloudera.org> on 2017/09/06 21:58:14 UTC

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

John Russell has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7983

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................

[DOCS] Explain Boost setting needed for 96-bit timestamps

People writing C++ UDFs need to compile with
-DBOOST_DATE_TIME_POSIX_TIME_STD_CONFIG to get
96-bit timestamps instead of 64-bit ones.

Added this caveat under both the TIMESTAMP reference
page, and the UDF page.

Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
---
M docs/shared/impala_common.xml
M docs/topics/impala_timestamp.xml
M docs/topics/impala_udf.xml
3 files changed, 20 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/7983/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/7983 )

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2:

If we're not going to move this forward can we abandon it?


-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-Change-Number: 7983
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Oct 2017 22:13:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change.

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2:

Tim and Michael, I added you as reviewers based on your frequent activity in be/src/exprs/. If someone else is more familiar with the compilation aspect w.r.t. TIMESTAMP, please suggest any other reviewer.

-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2: Code-Review-1

I think that the documented solution is not the correct solution to the problem. It may work by coincidence but relies on some very brittle assumptions about Impala exporting boost functions that have binary compatibility with the version of boost used by the UDF.

In future we will likely remove, update or modify the version of boost used in Impala.

-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has abandoned this change. ( http://gerrit.cloudera.org:8080/7983 )

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Abandoned

Suggested workaround was not a robust or future-proof solution.
-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-Change-Number: 7983
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change.

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2:

Patch set 2 is a no-op from a review perspective. I got rid of a commit that got into my local master branch by accident.

-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "John Russell (Code Review)" <ge...@cloudera.org>.
John Russell has posted comments on this change. ( http://gerrit.cloudera.org:8080/7983 )

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2:

Let me see about looping in the original requester who probably isn't looking at this gerrit. I was hoping for some more back-and-forth ending in a clear decision about how to handle suboptimal-but-what-we-have-today details w.r.t. UDFs.


-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-Change-Number: 7983
Gerrit-PatchSet: 2
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@apache.org>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Oct 2017 22:30:57 +0000
Gerrit-HasComments: No

Re: [Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by Tim Armstrong <ta...@cloudera.com>.
I've been talking to John out of band about this. I had some more meta
concerns about documenting this and whether it will encourage customers to
do dangerous things. Need to organise my thoughts and enumerate concerns.

On 12 Sep. 2017 9:42 am, "Matthew Jacobs (Code Review)" <ge...@cloudera.org>
wrote:

> Matthew Jacobs has posted comments on this change.
>
> Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
> ......................................................................
>
>
> Patch Set 2:
>
> (3 comments)
>
> http://gerrit.cloudera.org:8080/#/c/7983/2/docs/shared/impala_common.xml
> File docs/shared/impala_common.xml:
>
> PS2, Line 848: our
> who is 'our' here?
>
>
> PS2, Line 850: ,
>              :         which matches the representation for
> <codeph>TIMESTAMP</codeph> in Impala
> , and is required in order to use TIMESTAMP.
>
>
> Line 855: <codeblock>
> it might be good to just say that you should probably just define this
> variable always, unless you have a really good reason not to
>
>
> --
> To view, visit http://gerrit.cloudera.org:8080/7983
> To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
>
> Gerrit-MessageType: comment
> Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
> Gerrit-PatchSet: 2
> Gerrit-Project: Impala-ASF
> Gerrit-Branch: master
> Gerrit-Owner: John Russell <jr...@cloudera.com>
> Gerrit-Reviewer: John Russell <jr...@cloudera.com>
> Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
> Gerrit-Reviewer: Michael Ho
> Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
> Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
> Gerrit-HasComments: Yes
>

[Impala-ASF-CR] [DOCS] Explain Boost setting needed for 96-bit timestamps

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: [DOCS] Explain Boost setting needed for 96-bit timestamps
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7983/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

PS2, Line 848: our
who is 'our' here?


PS2, Line 850: ,
             :         which matches the representation for <codeph>TIMESTAMP</codeph> in Impala
, and is required in order to use TIMESTAMP.


Line 855: <codeblock>
it might be good to just say that you should probably just define this variable always, unless you have a really good reason not to


-- 
To view, visit http://gerrit.cloudera.org:8080/7983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4b67cd7762f682c3a054e0d9641080aa51801c83
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: John Russell <jr...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes