from:"anujphadke \(Code Review\)"

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

2018-10-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11540 )

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
..


Patch Set 3:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@7
PS1, Line 7: meout
> Rephrase to state what the change does, not what it should do, e.g. "Set so
Done


http://gerrit.cloudera.org:8080/#/c/11540/1//COMMIT_MSG@19
PS1, Line 19: 2. Created a kerberized impala cluster with ssl enabled and 
connected
> This should be automated, too.
1. I am not sure if it's easy to automate this test on a minicluster setup 
(kerberos + SSL)
2. I ran  test_client_ssl.py and created a minicluster setup with SSL enabled.  
Tried to block the beeswax server with telnet  or with an openssl client,  but 
it did not block the beeswax server (with just SSL enabled.)


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@81
PS1, Line 81: client_connect_timeout
> Is this milliseconds? If so, please rename to client_connect_timeout_ms
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@254
PS1, Line 254: sock, self.transport = self._get_socket_and_transport()
 : sock.setTimeout(self.client_connect_t
> You can write this in a single line:
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@257
PS1, Line 257: protocol = TBinaryProtocol.TBinaryProtocol(self.transport)
 : self.imp_service = ImpalaService.Client(protocol)
 : result = self.ping_impala_service()
 : sock.setTimeout(None)
> Which of these 4 lines can time out? Which of the ones that can time out do
Let me get back to you shortly on this one.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@261
PS1, Line 261: self.connected = True
> This could be more clear if we pass 0 instead
Does not work as expected if we pass 0
Here is the definition in -

build/impala-shell-3.1.0-SNAPSHOT/lib/thrift/transport/TSocket.py


def setTimeout(self, ms):
if ms is None:
  self.__timeout = None
else:
  self.__timeout = ms / 1000.0


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@274
PS1, Line 274:
> update the comment
Updated the comment to mention what this function returns.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_client.py@326
PS1, Line 326:   return sock, TSaslClientTransport(sasl_factory, "GSSAPI", 
sock)
> This will now return a tuple in some cases, and a transport in others. Plea
oops. I missed returning from this inner function. Let me make sure I test this 
part too.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell.py@864
PS1, Line 864: self.imp_client.close_connection()
> Please add a test that calls connect from the shell instead of passing a ho
Working on it. Will send a new patch with this change.


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py
File shell/impala_shell_config_defaults.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/impala_shell_config_defaults.py@54
PS1, Line 54: 'client_connect_timeout_ms': 5000,
> How did you decide on 60s? Why not pick something smaller, e.g. 5s?
I tried to keep this value large by default for secure connections to establish.
I searched around a bit after reading your comment and I  think 5 secs should 
suffice for a connection to be established. Do you know of any tests to check 
if 5secs would be good enough?


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py
File shell/option_parser.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@225
PS1, Line 225: client_connect_timeout
> client_connect_timeout_ms?
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/shell/option_parser.py@226
PS1, Line 226: "
> flake8: E251 unexpected spaces around keyword / parameter equals
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@737
PS1, Line 737: est_
> nit: Tests
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@743
PS1, Line 743: """
> Use contextlib.closing here: https://docs.python.org/2/library/contextlib.h
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@744
PS1, Line 744: n
> flake8: E231 missing whitespace after ','
Done


http://gerrit.cloudera.org:8080/#/c/11540/1/tests/shell/test_shell_commandline.py@745
PS1, Line 745: (
> Please add a comment what this 1 does?

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

2018-10-01 Thread anujphadke (Code Review)

Hello Thomas Marshall, Lars Volker, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
..

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect :21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 37 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11540/3
--
To view, visit http://gerrit.cloudera.org:8080/11540
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall

[Impala-ASF-CR] [WIP] IMPALA-7555: Set socket timeout in impala-shell

2018-10-01 Thread anujphadke (Code Review)

Hello Thomas Marshall, Lars Volker, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: [WIP] IMPALA-7555: Set socket timeout in impala-shell
..

[WIP] IMPALA-7555: Set socket timeout in impala-shell

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect :21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 38 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/11540/2
--
To view, visit http://gerrit.cloudera.org:8080/11540
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Thomas Marshall

[Impala-ASF-CR] [WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

2018-09-28 Thread anujphadke (Code Review)

anujphadke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11540


Change subject: [WIP] IMPALA-7555: Impala-shell should timeout if it fails to 
connect
..

[WIP] IMPALA-7555: Impala-shell should timeout if it fails to connect

impala-shell does not set any socket timeout while connecting to the
impala server. This change sets a timeout on the socket before
connecting and unsets it back after successfully connecting. The default
timeout on this socket is 1 min.
Usage: impala-shell --client_connect_timeout=

Testing:
1. Added a test where I create a random listening socket.
impala-shell connects to this socket and times out after the default
timeout value.
2. Created a kerberized impala cluster with ssl enabled and connected
to the impalad using an openssl client (block the beeswax server thread
to accept new connection) -
E.g. - openssl s_client -connect :21000
Used impala-shell to connect to the same impalad later. impala-shell
timed out after the default of 1 min.I verified it manually.

Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
---
M shell/impala_client.py
M shell/impala_shell.py
M shell/impala_shell_config_defaults.py
M shell/option_parser.py
M tests/shell/test_shell_commandline.py
5 files changed, 33 insertions(+), 9 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I130fc47f7a83f591918d6842634b4e5787d00813
Gerrit-Change-Number: 11540
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-7242/IMPALA-7243: check for overflow when converting IntVal to DecimalValue

2018-08-06 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11020 )

Change subject: IMPALA-7242/IMPALA-7243: check for overflow when converting 
IntVal to DecimalValue
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11020/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/11020/1/be/src/exprs/expr-test.cc@5382
PS1, Line 5382:   // IMPALA-7242/IMPALA-7243: check for overflow when 
converting IntVal to DecimalValue
> Can you mention the JIRA number here for context?
Done


http://gerrit.cloudera.org:8080/#/c/11020/1/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/11020/1/be/src/exprs/math-functions-ir.cc@541
PS1, Line 541: stringstream error_msg;
> Just to be clear, this one would be nice but is optional since it's consist
Having issues building impala locally to verify.
I have fixed the comment. I will send a follow up patch and replace this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5808802096bb0e6cfd4ab6464538474e42cab60
Gerrit-Change-Number: 11020
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 06 Aug 2018 21:32:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7242/IMPALA-7243: check for overflow when converting IntVal to DecimalValue

2018-08-06 Thread anujphadke (Code Review)

Hello Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: IMPALA-7242/IMPALA-7243: check for overflow when converting 
IntVal to DecimalValue
..

IMPALA-7242/IMPALA-7243: check for overflow when converting IntVal to 
DecimalValue

This change adds a check for an overflow which can happen during
converting num_buckets from IntVal to a DecimalValue during width_bucket
evaluation. An overflow can result in hitting a DCHECK if it needs int256
for intermediate evaluations or returns a wrong result otherwise.

Change-Id: Ie5808802096bb0e6cfd4ab6464538474e42cab60
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
2 files changed, 17 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/11020/2
--
To view, visit http://gerrit.cloudera.org:8080/11020
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5808802096bb0e6cfd4ab6464538474e42cab60
Gerrit-Change-Number: 11020
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-5695: restart first impalad() does not start impalad

2018-07-24 Thread anujphadke (Code Review)

anujphadke has abandoned this change. ( http://gerrit.cloudera.org:8080/9619 )

Change subject: IMPALA-5695: restart_first_impalad() does not start impalad
..


Abandoned

This is not the right fix. Will send a new patch.
The problem is that we don't wait for the restarted process anywhere
When a process is killed, it will become zombile until its parent waits for it
When you restart using start-impala-cluster.py, it doesn't have this problem, 
because the parent process is start-impala-cluster.py, which immediately exits 
after the cluster starts.
--
To view, visit http://gerrit.cloudera.org:8080/9619
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I30a5c25e132deebb9731891364196af31e57d875
Gerrit-Change-Number: 9619
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-29 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#21).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
7 files changed, 246 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/21
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 21
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-28 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 20:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/20/be/src/exprs/math-functions-ir.cc@565
PS20, Line 565:   ctx->SetError("Encountered division by 0");
clang fails with division by 0. Since we check for
We have a check above min_range >= max_range on line 519. Can't think of a case 
to hit this.
I added this to get pass the clang failure.
@taras - Can you take an another look? Thanks!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 20
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 29 Jun 2018 00:30:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-28 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#20).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
7 files changed, 254 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/20
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 20
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-28 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht, 
Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#19).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
7 files changed, 254 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/19
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 19
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-22 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 16:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6023/15/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/6023/15/be/src/exprs/expr-test.cc@582
PS15, Line 582:
> Can you document briefly what the function does?
Done


http://gerrit.cloudera.org:8080/#/c/6023/15/be/src/exprs/expr-test.cc@587
PS15, Line 587:   void TestError(const string& expr) {
> ASSERT_FALSE(status.ok())
Done


http://gerrit.cloudera.org:8080/#/c/6023/15/be/src/exprs/expr-test.cc@588
PS15, Line 588: GetValue(expr, INVALID_TYPE, /* expect_error */ true);
> This is basically EndsWith(), right? Can you factor out into a utility in S
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 16
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 22 Jun 2018 23:39:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-22 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Tim Armstrong, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#16).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M be/src/util/string-util.cc
M be/src/util/string-util.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
7 files changed, 246 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/16
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 16
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-06-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

(1 comment)

After rebasing I am seeing a few test failures because of a particular commit -

https://github.com/cloudera/Impala/blob/cdh5-trunk/fe/src/main/java/org/apache/impala/analysis/Expr.java#L513

If the input (children type is non decimal it throws an exception because we 
hit the precondition. The chilType is a TinyInt in this case.

[localhost:21000] default> select width_bucket(NULL, 5, 20.1, 4);
Query: select width_bucket(NULL, 5, 20.1, 4)
Query submitted at: 2018-06-19 15:10:25 (Coordinator: 
http://anuj-OptiPlex-9020:25000)
ERROR: IllegalStateException: null

Works fine this way -
Query: select width_bucket(NULL, 5.0, 20.1, 4)
Query submitted at: 2018-06-19 15:10:21 (Coordinator: 
http://anuj-OptiPlex-9020:25000)
Query progress can be monitored at: 
http://anuj-OptiPlex-9020:25000/query_plan?query_id=c8413395d2645a21:d1ff71de
+--+
| width_bucket(null, 5.0, 20.1, 4) |
+--+
| NULL |
+--+
Fetched 1 row(s) in 0.11s

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java@490
PS15, Line 490:   result = ((ScalarType)result).getMinResolutionDecimal();
> When I implemented this using decimalVal. I found a bug.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 19 Jun 2018 23:56:15 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

2018-06-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10740 )

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc
File be/src/util/network-util.cc:

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc@196
PS2, Line 196: server_address.sin_port = htons(port);
I think we can set this to 0 and bind to a random port instead of looping 
through to find a random port?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 19 Jun 2018 23:05:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7179: allow multiple scratch dirs per device=true by default

2018-06-15 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10736 )

Change subject: IMPALA-7179: allow_multiple_scratch_dirs_per_device=true by 
default
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23394c9949ae4cd0a21d7bb25551371b3198e76c
Gerrit-Change-Number: 10736
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 15 Jun 2018 23:25:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-05-25 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java@490
PS15, Line 490:   result = ((ScalarType)result).getMinResolutionDecimal();
> Why do we need this?
When I implemented this using decimalVal. I found a bug.
Without this change -
[localhost:21000] > select width_bucket(29 , 1, 400, 7);
Query: select width_bucket(29 , 1, 400, 7)
Query submitted at: 2017-02-28 18:41:40 (Coordinator: 
http://anuj-OptiPlex-9020:25000)
ERROR: AnalysisException: null
CAUSED BY: IllegalStateException: null

I debugged it and found that
getREsolvedWildCardType() does not return a decimalType for this specific 
sequence when
child(0) -> TinyInt
chile(1) -> tinyInt
child(2) -> smalInt

result = Type.getAssignmentCompatibleType(result, childType, false);

when the result is Decimal(3,0) and childType is SMALLINT the result  it 
returns is a result which is SMALLINT.

The query hits this check and fails -
Preconditions.checkState(result.isDecimal() && !result.isWildcardDecimal());

Had discussed this with Alex and he mentioned that we can fix this as part of 
this change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 26 May 2018 03:59:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-05-08 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6023/13/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/6023/13/be/src/exprs/expr-test.cc@5331
PS13, Line 5331: ", 40)", TYPE_BIGINT, 1);
> We should test the two conditions that can lead to requiring int256_t separ
Yes,
I tested individual scenarios by having 2 separate loops. I dont have the exact 
change. But something like this.
Made sure we hit only one case is true.
+  if(BitUtil::CountLeadingZeros(abs(buckets.value())) +
+BitUtil::CountLeadingZeros(abs(dist_from_min.value())) <= 128) {
+
+   log(INFO)<< "Overflow while multiplying buckets.value * 
dist_from_min.value")";
+  }
+
+  if(BitUtil::CountLeadingZeros(range_size.value()) +
+detail::MinLeadingZerosAfterScaling(BitUtil::CountLeadingZeros(
+  range_size.value()), input_scale) <= 128) {
+
+LOG(INFO) << "Overflow while scaling up range size";
+  }
+


http://gerrit.cloudera.org:8080/#/c/6023/13/be/src/exprs/expr-test.cc@5333
PS13, Line 5333:   // evaluated with int128_t. Incrementing one of them will 
require using int256_t for
> Maybe add a few more cases where int256_t is used? I would add these tests:
Done.

smallest values where int256_t is needed and there should be an overflow -
Since the result of the subtraction dist_from_min =  (expr - min_val) is stored 
in decimal16Val
 Since, dist_from_min * num_buckets might need to be stored in int256_t to 
avoid overflowing int128_t.
Multiplying  Decimal16Val by intVal should not cause overflows when we store 
results in  int256_t



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 09 May 2018 00:01:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-05-08 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#15).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 232 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/15
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-04-02 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#13).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 221 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/13
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 13
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-03-27 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 12:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@505
PS10, Line 505: f
> Suppose min_range = -... and max_range=..., so the subtraction over
I am fine with it. I have moved the check above.


http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/math-functions-ir.cc@502
PS11, Line 502:
  :   Decimal16Val
> nit: why not combine the two lines?
Done


http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/math-functions-ir.cc@524
PS11, Line 524:
> Why do we need to declare total_leading_zeros here? Also, make this UNLIKEL
Done


http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/math-functions-ir.cc@529
PS11, Line 529: e
> nit: capital letter
Done


http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/math-functions-ir.cc@531
PS11, Line 531: return result;
> You should use MinLeadingZerosAfterScaling() here. Also, make this UNLIKELY
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 12
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 27 Mar 2018 20:59:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-03-27 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#12).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 224 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/12
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 12
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-5695: restart first impalad() does not start impalad

2018-03-15 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9619 )

Change subject: IMPALA-5695: restart_first_impalad() does not start impalad
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9619/1/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/9619/1/tests/authorization/test_grant_revoke.py@80
PS1, Line 80: IMPALA_HOME = os.environ['IMPALA_HOME']
: IMPALAD_PATH = os.path.join(IMPALA_HOME,
:'bin/start-impalad.sh --server_name=server1')
> The issue is that impalad.restart() remains broken.  I think the proper fix
Thanks Adam and Freddy!
I agree. I looked into it and the issue does not happen if you just set 
shell=True in the popen in exec_process_async.
I am still digging up the code to root cause why this works. I will post my 
updates soon.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30a5c25e132deebb9731891364196af31e57d875
Gerrit-Change-Number: 9619
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 16 Mar 2018 02:24:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5695: restart first impalad() does not start impalad

2018-03-14 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9619 )

Change subject: IMPALA-5695: restart_first_impalad() does not start impalad
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9619/1/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/9619/1/tests/authorization/test_grant_revoke.py@80
PS1, Line 80: IMPALA_HOME = os.environ['IMPALA_HOME']
: IMPALAD_PATH = os.path.join(IMPALA_HOME,
:'bin/start-impalad.sh --server_name=server1')
> Could we get this data from /proc//environ as part of refresh?
The restart method in impala_cluster.py seems to be extracting this path from 
psutil.Process. The issue in doing this is it wont set the classpath correctly 
and impalad will exit out failing to find JNI.

The restart method defined in Process seems to be more generic. It uses cmdline 
to start the process. So I didn't want to hardcode this script path there.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I30a5c25e132deebb9731891364196af31e57d875
Gerrit-Change-Number: 9619
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Mar 2018 22:41:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5695: restart first impalad() does not start impalad

2018-03-13 Thread anujphadke (Code Review)

anujphadke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9619


Change subject: IMPALA-5695: restart_first_impalad() does not start impalad
..

IMPALA-5695: restart_first_impalad() does not start impalad

restart_first_impalad() does not set the correct classpath before
starting impalad. This results in impalad exiting as it fails to
find the JniUtil class. This change invokes the start_impalad.sh
to start impalad.

Testing:
test_role_update consistently fails without this change.

Change-Id: I30a5c25e132deebb9731891364196af31e57d875
---
M tests/authorization/test_grant_revoke.py
1 file changed, 12 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I30a5c25e132deebb9731891364196af31e57d875
Gerrit-Change-Number: 9619
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-02 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions-ir.cc@121
PS1, Line 121:   v.val * pow(10.0, scale.val) + ((v.val < 0) ? -0.5 : 0.5)) 
/ pow(10.0, scale.val));
Should we also check for overflows here , set FunctionContext with an error and 
return a NULL?
ctx->SetError

Found a couple of possible inconsistencies (without your change). If your 
change fixes it we can add these tests?

[nightly-unsecure-2.gce.cloudera.com:21000] > select dround(cast(2.51 as 
double), 256);
Query: select dround(cast(2.51 as double), 256)
Query submitted at: 2018-03-02 16:44:05 (Coordinator: 
http://nightly-unsecure-2.gce.cloudera.com:25000)
Query progress can be monitored at: 
http://nightly-unsecure-2.gce.cloudera.com:25000/query_plan?query_id=c5421271ce74b797:4da87021
+---+
| dround(cast(2.51 as double), 256) |
+---+
| 2.51  |
+---+
Fetched 1 row(s) in 0.01s
[nightly-unsecure-2.gce.cloudera.com:21000] > select dround(cast(2.51 as 
double), 257);
Query: select dround(cast(2.51 as double), 257)
Query submitted at: 2018-03-02 16:44:09 (Coordinator: 
http://nightly-unsecure-2.gce.cloudera.com:25000)
Query progress can be monitored at: 
http://nightly-unsecure-2.gce.cloudera.com:25000/query_plan?query_id=d4969d969dba34a:639969f
+---+
| dround(cast(2.51 as double), 257) |
+---+
| 2.511 |
+---+
Fetched 1 row(s) in 0.01s

[nightly-unsecure-2.gce.cloudera.com:21000] > select dround(cast(2.51 as 
double), 400);
Query: select dround(cast(2.51 as double), 400)
Query submitted at: 2018-03-02 16:50:01 (Coordinator: 
http://nightly-unsecure-2.gce.cloudera.com:25000)
Query progress can be monitored at: 
http://nightly-unsecure-2.gce.cloudera.com:25000/query_plan?query_id=c741e669098c64a0:d7514ce1
+---+
| dround(cast(2.51 as double), 400) |
+---+
| NaN   |
+---+
Fetched 1 row(s) in 0.01s



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:58:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

2018-03-02 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 1:

impalad crashes with this patch at startup.

I0302 16:29:52.235250 19443 status.cc:125] Unable to find 
_ZN6impala13MathFunctions9RoundUpToEPN10impala_udf15FunctionContextERKNS1_9DoubleValERKNS1_9BigIntValE
dlerror: /home/anuj/Impala/be/build/latest/service/impalad: undefined symbol: 
_ZN6impala13MathFunctions9RoundUpToEPN10impala_udf15FunctionContextERKNS1_9DoubleValERKNS1_9BigIntValE
@  0x166cd33  impala::Status::Status()
@  0x1a664ec  impala::DynamicLookup()
@  0x182e579  impala::LibCache::GetSoFunctionPtr()
@  0x182ef5a  impala::LibCache::CheckSymbolExists()
@  0x193a533  ResolveSymbolLookup()
@  0x193b00b  
Java_org_apache_impala_service_FeSupport_NativeLookupSymbol
@ 0x7fb3508edc39  (unknown)
F0302 16:29:52.236079 19443 frontend.cc:108] 
java.lang.ExceptionInInitializerError
  at org.apache.impala.service.BackendConfig.create(BackendConfig.java:47)
  at org.apache.impala.service.JniFrontend.(JniFrontend.java:123)
Caused by: java.lang.RuntimeException: Builtin symbol 
'impala::MathFunctions::RoundUpTo'[DOUBLE, BIGINT] not found!
  at 
org.apache.impala.catalog.ScalarFunction.createBuiltin(ScalarFunction.java:82)
  at org.apache.impala.catalog.Db.addScalarBuiltin(Db.java:393)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:35:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

2018-03-02 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9300/6/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/9300/6/be/src/common/init.cc@240
PS6, Line 240:   signal(SIGPIPE, SIG_IGN);
Could ignoring SIGPIPE have any side effects?

E.g -
IMPALA-5187 depends on Breakpad #728:
When writing a minidump on Linux, Breakpad called clone() in
linux/handler/exception_handler.cc with the CLONE_FILES flag. If the
parent process died while the child waited for the continuation signal,
the write side of the pipe 'fdes' stayed open in the child. The child
would not receive a SIGPIPE and would wait forever.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 6
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 02 Mar 2018 22:29:13 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-21 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9154 )

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9154/5/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/9154/5/be/src/codegen/llvm-codegen.cc@220
PS5, Line 220:   status = (*codegen)->Init(std::move(loaded_module));
> We should still need to close *codegen if this fails. I.e. I think we need
Done


http://gerrit.cloudera.org:8080/#/c/9154/5/be/src/codegen/llvm-codegen.cc@251
PS5, Line 251:   module_name, _module);
> Same comment applies here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 6
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 22 Feb 2018 04:54:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-21 Thread anujphadke (Code Review)

Hello Tim Armstrong, Bikramjeet Vig,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#6).

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols()
fails and returns on error and does not close the codegen object. This
patch closes the codegen object on failure. This avoids hitting a DCHECK
later up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/bad_udf.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/bad_udf.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/bad_udf.ll
LLVM diagnostic error: Invalid bitcode signature

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
M testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
M tests/query_test/test_udfs.py
3 files changed, 48 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/9154/6
--
To view, visit http://gerrit.cloudera.org:8080/9154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 6
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-02-21 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 11:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/6023/11/be/src/exprs/expr-test.cc@585
PS11, Line 585: 
ASSERT_TRUE(status.msg().msg().compare(status.msg().msg().size() -
status.msg().msg() is prepended by SQLSTATE_GENERAL_ERROR string. I am using to 
compare to check if status.msg().msg() ends with the error string.
https://github.com/apache/impala/blob/19ab465b391167e99658c9270a2a3d76b2b2652f/be/src/service/impala-server.cc#L231


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@438
PS10, Line 438: num_buc
> nit: should be num_buckets instead of buckets
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@444
PS10, Line 444: result
> nit: "results"
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@455
PS10, Line 455: a
> nit: "an overflow"
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@482
PS10, Line 482:   if (UNLIKELY(min_range >= max_range)) {
> UNLIKELY
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@487
PS10, Line 487:   if (UNLIKELY(expr < min_range)) return 0;
> UNLIKELY
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@489
PS10, Line 489:   if (UNLIKELY(expr >= max_range)) {
> UNLIKELY
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@504
PS10, Line 504:   input_scale, input_precision, input_scale, false, 
);
> UNLIKELY(overflow)
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@505
PS10, Line 505: f
> I think this check needs to happen above the line 487. Imagine if this over
If expr < min_range, we return 0
so If I move this above l487 it will start returning null if max_range - 
min_range overflows.
If  expr < min_range why should we perform this subtraction if the bucket 
number can be evaluated without performing this subtraction


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@507
PS10, Line 507: error_msg << "Overflow while evaluating the difference 
between min_range: " <<
> Add an end to end test for this error message (and others).
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@518
PS10, Line 518:   Decimal16Value buckets = 
Decimal16Value::FromInt(input_precision, input_scale,
> Add an end to end test for this error message.
Since we won't hit this without overflowing above.
Added a test  where expr - min will overflow, but we detect the  overflow 
during subtraction above and return.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@536
PS10, Line 536:
> I am not convinced this check is correct. In which case will this check be
Removed and added the leading zero check.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@536
PS10, Line 536:
> I am not convinced this check is correct. In which case will this check be
Removed this and just added the leading zero check.


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@538
PS10, Line 538:   if (needs_int256) {
> Is it possible to tell statically that needs_int256 is false? For example,
Added the leading zero check to detect if we need int256. Would it suffice?


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions-ir.cc@566
PS10, Line 566: const DecimalVal& min_range, const DecimalVal& max_range,
> We need to DCHECK that it did not overflow here. (It should be easy to do i
Done


http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions.h
File be/src/exprs/math-functions.h:

http://gerrit.cloudera.org:8080/#/c/6023/10/be/src/exprs/math-functions.h@115
PS10, Line 115:   static BigIntVal WidthBucket(FunctionContext* ctx, const 
DecimalVal& expr,
> Does this need to be declared in this file? Isn't it an internal function?
I made this private. Not sure, any pointers where I can move this function to?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 11
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer:

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-02-21 Thread anujphadke (Code Review)

Hello Taras Bobrovytsky, Michael Brown, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#11).

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..

IMPALA-4848: Add WIDTH_BUCKET() function

Syntax :
width_bucket(expr decimal, min_val decimal, max_val decimal,
  num_buckets int)

This function creates equiwidth histograms , where the histogram range
is divided into num_buckets buckets having identical sizes. This
function returns the bucket in which the expr value would fall. min_val
and max_val are the minimum and maximum value of the histogram range
respectively.

-> This function returns NULL if expr is a NULL.
-> This function returns 0 if expr < min_val
-> This function returns num_buckets + 1 if expr > max_val

 E.g.
[localhost:21000] > select width_bucket(8, 1, 20, 3);
+---+
| width_bucket(8, 1, 20, 3) |
+---+
| 2 |
+---+

Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
---
M be/src/exprs/expr-test.cc
M be/src/exprs/math-functions-ir.cc
M be/src/exprs/math-functions.h
M common/function-registry/impala_functions.py
M fe/src/main/java/org/apache/impala/analysis/Expr.java
5 files changed, 227 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/6023/11
--
To view, visit http://gerrit.cloudera.org:8080/6023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 11
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-21 Thread anujphadke (Code Review)

Hello Tim Armstrong, Bikramjeet Vig,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols()
fails and returns on error and does not close the codegen object. This
patch closes the codegen object on failure. This avoids hitting a DCHECK
later up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/bad_udf.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/bad_udf.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/bad_udf.ll
LLVM diagnostic error: Invalid bitcode signature

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
M testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
M tests/query_test/test_udfs.py
3 files changed, 42 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/9154/5
--
To view, visit http://gerrit.cloudera.org:8080/9154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 5
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-21 Thread anujphadke (Code Review)

Hello Tim Armstrong, Bikramjeet Vig,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols()
fails and returns on error and does not close the codegen object. This
patch closes the codegen object on failure. This avoids hitting a DCHECK
later up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/bad_udf.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/bad_udf.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/bad_udf.ll
LLVM diagnostic error: Invalid bitcode signature

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
M testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
M tests/query_test/test_udfs.py
3 files changed, 39 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/9154/4
--
To view, visit http://gerrit.cloudera.org:8080/9154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 4
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6516: Log catalog update only if the catalog version changes

2018-02-13 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9311 )

Change subject: IMPALA-6516: Log catalog update only if the catalog version 
changes
..


Patch Set 1: Code-Review+1

Thanks for fixing this. My impalad.INFO was flooded with these messages.
update applied with version: 1095 new min catalog object version: 2
I0213 15:29:33.942580  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:35.944761  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:37.946624  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:39.948545  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2
I0213 15:29:41.950410  9332 impala-server.cc:1354] Catalog topic update applied 
with version: 1095 new min catalog object version: 2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I04b8dd05c588d4cd91e9ca2251f8f66325bb45e2
Gerrit-Change-Number: 9311
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 14 Feb 2018 00:28:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-09 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9154 )

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9154/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9154/2//COMMIT_MSG@12
PS2, Line 12: fails an
> nit: fails and
Done


http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc@280
PS2, Line 280:  return Status(diagnostic_err);
> since the diagnostic handler is called, it would be worth adding the messag
Done


http://gerrit.cloudera.org:8080/#/c/9154/2/be/src/codegen/llvm-codegen.cc@1315
PS2, Line 1315: Status status = CreateFromFile(nullptr, , nullptr, file, 
module_id, );
  :   if (!status.ok()) {
  : codegen->Close();
  : return status;
  :   }
> i hit the same problem earlier when I tried loading a lib generated from an
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Sat, 10 Feb 2018 00:54:41 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-02-09 Thread anujphadke (Code Review)

Hello Bikramjeet Vig,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols()
fails and returns on error and does not close the codegen object. This
patch closes the codegen object on failure. This avoids hitting a DCHECK
later up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/bad_udf.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/bad_udf.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/bad_udf.ll
LLVM diagnostic error: Invalid bitcode signature

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
M testdata/workloads/functional-query/queries/QueryTest/udf-errors.test
M tests/query_test/test_udfs.py
3 files changed, 27 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/9154/3
--
To view, visit http://gerrit.cloudera.org:8080/9154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Bikramjeet Vig

[Impala-ASF-CR] IMPALA-6113: Skip row groups with predicates on NULL columns

2018-02-01 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9140 )

Change subject: IMPALA-6113: Skip row groups with predicates on NULL columns
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9140/3/tests/query_test/test_parquet_stats.py
File tests/query_test/test_parquet_stats.py:

http://gerrit.cloudera.org:8080/#/c/9140/3/tests/query_test/test_parquet_stats.py@49
PS3, Line 49: self.run_test_case('QueryTest/parquet-stats', vector, 
use_db=unique_database)
> Sorry for the churn. Why do you need to use $DATABASE though anyway? AFAIK
Sorry about the confusion. I thought that adding the create table statement in 
.test file won't create the table under a  unique_db.
I manually tried it later with your first patch and checked the code. It  
creates a unique db without the $DATABASE suffix. Verified this through the 
impala-shell while running the test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I141317af0e0df30da8f220b29b0bfba364f40ddf
Gerrit-Change-Number: 9140
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 01 Feb 2018 22:40:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6113: Skip row groups with predicates on NULL columns

2018-01-30 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9140 )

Change subject: IMPALA-6113: Skip row groups with predicates on NULL columns
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9140/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/9140/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@463
PS1, Line 463: create table table_for_null_count_test (i int, j int) stored as 
parquet;
> Hey Anuj, thanks for taking a look!
This is to avoid conflicts when running these tests in parallel. Looks like you 
have already moved it there. Thanks!
I actually tried running these tests from your old patch and it used the 
unique_db to create the tables without using the $DATABASE. Sorry about the 
noise.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I141317af0e0df30da8f220b29b0bfba364f40ddf
Gerrit-Change-Number: 9140
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 30 Jan 2018 18:54:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6450: loosen DCHECK in EventSequence::Start()

2018-01-29 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9155 )

Change subject: IMPALA-6450: loosen DCHECK in EventSequence::Start()
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I62317149cb8428f7d29e945a538f6cc8dd45342f
Gerrit-Change-Number: 9155
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 30 Jan 2018 00:12:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-01-29 Thread anujphadke (Code Review)

anujphadke has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/9154 )

Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols()
failsand returns on error and does not close the codegen object. This
patch closes the codegen object on failure. This avoids hitting a DCHECK
later up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

Testing:
Created a UDF using a shared object with a .ll extension (renamed a .so
object). This change returns the following error instead of crashing -

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/test.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/test.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/test.ll
Could not parse module /tmp/test.15137.0.ll: llvm.bitcode:2

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
1 file changed, 9 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/9154/2
--
To view, visit http://gerrit.cloudera.org:8080/9154
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-6008: Creating a UDF from a shared library with a .ll extenion crashes impala

2018-01-29 Thread anujphadke (Code Review)

anujphadke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9154


Change subject: IMPALA-6008: Creating a UDF from a shared library with a .ll 
extenion crashes impala
..

IMPALA-6008: Creating a UDF from a shared library with a .ll extenion
crashes impala

Impala crashes on creating a UDF from a shared library (.so file) which
was renamed to have .ll extension. CreateFile() call in GetSymbols() fails
and returns on error and does not close the codegen object. This patch
closes the codegen object on failure. This avoids hitting a DCHECK later
up in the stack.
The chain of failures also invokes the DiagnosticHandlerFn. RuntimeState
object is NULL when the DiagnosticHandlerFn gets called in this case.
This change also adds a check before accessing it for logging.

Testing:
Created a UDF using a shared object with a .ll extension (renamed a .so
object). This change returns the following error instead of crashing -

[localhost:21000] > create function foo4 (string, string) returns string
location '/tmp/test.ll' symbol='MyAwesomeUdf';
Query: create function foo4 (string, string) returns string location
'/tmp/test.ll' symbol='MyAwesomeUdf'
ERROR: AnalysisException: Could not load binary: /tmp/test.ll
Could not parse module /tmp/test.15137.0.ll: llvm.bitcode:2

Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
---
M be/src/codegen/llvm-codegen.cc
1 file changed, 9 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id060668802ca9c80367cdc0e8a823b968d549bbb
Gerrit-Change-Number: 9154
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-6311: Skip row groups with predicates on NULL columns

2018-01-25 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9140 )

Change subject: IMPALA-6311: Skip row groups with predicates on NULL columns
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9140/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/9140/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@463
PS1, Line 463: create table table_for_null_count_test (i int, j int) stored as 
parquet;
Can we modify these create table statements and move them
test_parquet_stats.py and create them using a unique database?


http://gerrit.cloudera.org:8080/#/c/9140/1/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@485
PS1, Line 485: create table table_for_null_count_test2 (i int, j int) stored as 
parquet;
same here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I141317af0e0df30da8f220b29b0bfba364f40ddf
Gerrit-Change-Number: 9140
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 25 Jan 2018 21:48:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9102 )

Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9102/3/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
File testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test:

http://gerrit.cloudera.org:8080/#/c/9102/3/testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test@38
PS3, Line 38:   select cast('a' as char(4)) as s from functional.alltypestiny
> Can you add a similar test for cast(NULL as char(4))? Or add it to this que
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 4
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 24 Jan 2018 01:40:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

Hello Philip Zeyliger, Tim Armstrong,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..

IMPALA-6435: Disable codegen for CHAR literals.

Currently we do not codegen CHAR types. This change checks
for CHAR literals in a expr and disables codegen.

Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
---
M be/src/exprs/literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 20 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/9102/4
--
To view, visit http://gerrit.cloudera.org:8080/9102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 4
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9102 )

Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9102/1/be/src/exprs/literal.cc
File be/src/exprs/literal.cc:

http://gerrit.cloudera.org:8080/#/c/9102/1/be/src/exprs/literal.cc@a392
PS1, Line 392:
> This seems wrong; you deleted the handlig for TYPE_STRING.
Ooops! Sorry I had removed some of the code to debug some test failure.
Forgot to git add it back again. Removed the TYPE_CHAR case here and I return 
earlier.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 23 Jan 2018 21:20:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

Hello Philip Zeyliger,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..

IMPALA-6435: Disable codegen for CHAR literals.

Currently we do not codegen CHAR types. This change checks
for CHAR literals in a expr and disables codegen.

Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
---
M be/src/exprs/literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 18 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/9102/3
--
To view, visit http://gerrit.cloudera.org:8080/9102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

Hello Philip Zeyliger,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..

IMPALA-6435: Disable codegen for CHAR literals.

Currently we do not codegen CHAR types. This change checks
for CHAR literals in a expr and disables codegen.

Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
---
M be/src/exprs/literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 22 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/9102/2
--
To view, visit http://gerrit.cloudera.org:8080/9102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Philip Zeyliger

[Impala-ASF-CR] IMPALA-6435: Disable codegen for CHAR literals.

2018-01-23 Thread anujphadke (Code Review)

anujphadke has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9102


Change subject: IMPALA-6435: Disable codegen for CHAR literals.
..

IMPALA-6435: Disable codegen for CHAR literals.

Currently we do not codegen CHAR types. This change checks
for CHAR literals in a expr and disables codegen.

Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
---
M be/src/exprs/literal.cc
M testdata/workloads/functional-query/queries/QueryTest/disable-codegen.test
2 files changed, 18 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e4e27350c53bc69ce412a004e392e7480214f73
Gerrit-Change-Number: 9102
Gerrit-PatchSet: 1
Gerrit-Owner: anujphadke

[Impala-ASF-CR] IMPALA-6422: Use ldexp() instead of powf() in HLL.

2018-01-19 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9078 )

Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL.
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9078/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9078/1//COMMIT_MSG@14
PS1, Line 14: HllFinalEstimate() where floating poing power of two
typo



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382
Gerrit-Change-Number: 9078
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 19 Jan 2018 17:49:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-01-16 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 10:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc@4653
PS9, Line 4653:   TestValue("is_nan(0.0)", TYPE_BOOLEAN, false);
> Add some tests with nulls.
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@455
PS9, Line 455: Subtracting a negative min_range from e
> You can just write:
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460: cimal8
> "In case"
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460: re the re
> function*
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460
PS9, Line 460: n
> extra space here
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461
PS9, Line 461: store the res
> I am not sure it makes sense to even mention decimal8Value here. Why would
It doesn't.
I was trying to detail out why everything needs to be stored as a 
decimal16Value (asked in PS5).
I was trying to convey that this cannot be stored in a decimal8value if it does 
not overflow.
I ll remove it since it is sounding confusing.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461
PS9, Line 461: ith the
> spelling
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@480
PS9, Line 480:   }
> What if min_range > max_range?
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@481
PS9, Line 481:
> Would it make sense to include the name of the function in the error messag
We don't in all the cases.
https://github.com/apache/incubator-impala/blob/master/be/src/exprs/string-functions-ir.cc#L338


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@489
PS9, Line 489: _range) {
> how about: num_buckets.val + 1
I am storing in num_buckets in a BigIntVal first and then incrementing to avoid 
the overflow issue.

result.val = num_buckets.val + 1 // will evaluate the RHS first and assign the 
overflowed value to result.

Adding 1 later after storing it in a BigIntVal to avoid this.

I have a test case -
TestValue("width_bucket(22, 5, 20.1, 2147483647)", TYPE_BIGINT, 2147483648)


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@494
PS9, Line 494:   }
> Maybe expand this comment a little? E.g
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@503
PS9, Line 503:   range_size = max_range.template 
Subtract(input_scale, min_range,
> Actually I'm not correct here. It does overflow in the case I provided.
We have a test case for this.
 // Test max - min will overflow during  width_bucket evaluation
  TestError("width_bucket(11, -9, 99, 
4000)");


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@514
PS9, Line 514:   dist_from_min = expr.template Subtract(input_scale, 
min_range, input_scale,
> Can this overflow ever happen without overflowing on line 503? Add a test c
Right, this overflow won't happen without overflowing the subtraction on l 503.
It's not needed. Should we have this extra check for safety?


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@533
PS9, Line 533:
> Nit: Put a space between "if" and the opening brace. (here and elsewhere)
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@539
PS9, Line 539:
> avoid written twice
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@549
PS9, Line 549: // match the scale of the numerator.
> Can you explain this a little better? It's not really clear why we have to
Done


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@551
PS9, Line 551: range_size.value()), input_scale);
> Is it possible for this to overflow?
Yes. I added a check for this and set needs_int256 to true if it overflows.
Added a a test for this.


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@557
PS9, Line 557:   return BigIntVal::null();
> This should be moved up to around line 543.
Moved it above.
This ideally should not overflow  since the result of the division is the 
bucket number which is a IntVal.
Are you fine with having this check for safety?


http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@571
PS9, Line 571: }
> Why add this check here, instead of around line 480 where other similar che
Done

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

2018-01-08 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8548 )

Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8548/2/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:

http://gerrit.cloudera.org:8080/#/c/8548/2/tests/query_test/test_insert_parquet.py@310
PS2, Line 310:   if line_split[0] == "id":
> Can you make this an elif chain and assert that the column name is one of t
Done


http://gerrit.cloudera.org:8080/#/c/8548/2/tests/query_test/test_insert_parquet.py@339
PS2, Line 339:   if line_split[0] == "id":
> Same here
Done


http://gerrit.cloudera.org:8080/#/c/8548/2/tests/query_test/test_insert_parquet.py@349
PS2, Line 349:
> nit: missing space after %
Done


http://gerrit.cloudera.org:8080/#/c/8548/2/tests/query_test/test_insert_parquet.py@351
PS2, Line 351: c
> same here
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 08 Jan 2018 22:57:48 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

2018-01-08 Thread anujphadke (Code Review)

Hello Tianyi Wang, Tim Armstrong,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..

IMPALA-5052: Read and write signed integer logical types in Parquet

This patch maps a signed integer logical type in parquet to a supported
Impala column type. This change introduces the following mapping -

  INT_8  -> TINYINT
  INT_16 -> SMALLINT
  INT_32 -> INT
  INT_64 -> BIGINT

Also, added a parquet file with the following schema for testing -

  schema {
optional int32 id;
optional int32 tinyint_col (INT_8);
optional int32 smallint_col (INT_16);
optional int32 int_col;
optional int64 bigint_col;
  }

Change-Id: I47a8371858c9597c6a440808cf6f933532468927
---
M be/src/exec/hdfs-parquet-table-writer.cc
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M testdata/data/README
A testdata/data/signed_integer_logical_types.parquet
M tests/query_test/test_insert_parquet.py
5 files changed, 99 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/8548/3
--
To view, visit http://gerrit.cloudera.org:8080/8548
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 3
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke

[Impala-ASF-CR] IMPALA-3833: Fix invalid data handling in Sequence and RCFile scanners

2018-01-05 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8936 )

Change subject: IMPALA-3833: Fix invalid data handling in Sequence and RCFile 
scanners
..


Patch Set 2:

Can we test all the cases fixed this with 100% reproducible cases?
For Ex:
I commented out the change in  decompress.cc and ran the fuzz test and it 
passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9cfc38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 8936
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 05 Jan 2018 20:45:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

2018-01-04 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8548 )

Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8548/1/testdata/data/signed_integer_logical_types.parquet
File testdata/data/signed_integer_logical_types.parquet:

PS1:
> Can you add a description of this file to the readme - i.e. what it has in
Done


http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py
File tests/query_test/test_insert_parquet.py:

http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@295
PS1, Line 295: column
> column
Done


http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@303
PS1, Line 303: stored as parquet""".format(src_tbl, hdfs_path)
> Why is there a space after {1}?
Removed


http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@321
PS1, Line 321: result = self.execute_query_expect_success(self.client, 
insert_stmt)
> remove semicolon
Done


http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@325
PS1, Line 325: ame values in
> should these (above and below as well) be execute_query_expect_success?
Done


http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@331
PS1, Line 331: dst_tbl = "{0}.{1}".format(unique_database, 
"read_write_logical_type_dst")
> +1. I think the test would be a little easier to understand too if we asser
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Fri, 05 Jan 2018 02:45:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5052: Read and write signed integer logical types in Parquet

2018-01-04 Thread anujphadke (Code Review)

Hello Tianyi Wang, Tim Armstrong,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: IMPALA-5052: Read and write signed integer logical types in 
Parquet
..

IMPALA-5052: Read and write signed integer logical types in Parquet

This patch maps a signed integer logical type in parquet to a supported
Impala column type. This change introduces the following mapping -

  INT_8  -> TINYINT
  INT_16 -> SMALLINT
  INT_32 -> INT
  INT_64 -> BIGINT

Also, added a parquet file with the following schema for testing -

  schema {
optional int32 id;
optional int32 tinyint_col (INT_8);
optional int32 smallint_col (INT_16);
optional int32 int_col;
optional int64 bigint_col;
  }

Change-Id: I47a8371858c9597c6a440808cf6f933532468927
---
M be/src/exec/hdfs-parquet-table-writer.cc
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M testdata/data/README
A testdata/data/signed_integer_logical_types.parquet
M tests/query_test/test_insert_parquet.py
5 files changed, 94 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/8548/2
--
To view, visit http://gerrit.cloudera.org:8080/8548
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Gerrit-Change-Number: 8548
Gerrit-PatchSet: 2
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2017-11-15 Thread anujphadke (Code Review)

anujphadke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6023/7/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/6023/7/be/src/exprs/math-functions-ir.cc@429
PS7, Line 429: bucket_width
> Done
Please ignore my last comment. Accidentally got posted when replying to Dan's 
comment. I still havn't uploaded my latest patch.


http://gerrit.cloudera.org:8080/#/c/6023/7/be/src/exprs/math-functions-ir.cc@431
PS7, Line 431: width_size
> Done
Please ignore my last comment. Accidentally got posted when replying to Dan's 
comment. I still havn't uploaded my latest patch.


http://gerrit.cloudera.org:8080/#/c/6023/7/be/src/exprs/math-functions-ir.cc@479
PS7, Line 479: result.val = num_buckets.val;
> Done
Please ignore my last comment. Accidentally got posted when replying to Dan's 
comment. I still haven't uploaded my latest patch.


http://gerrit.cloudera.org:8080/#/c/6023/7/be/src/exprs/math-functions-ir.cc@516
PS7, Line 516:   int256_t x = ConvertToInt256(buckets.value()) * 
ConvertToInt256(width_size.value());
> This patch stores intermediate results in int256_t only when needed. Uses i
Please ignore my last comment. Accidentally got posted when replying to Dan's 
comment. I still havn't uploaded my latest patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 7
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Thu, 16 Nov 2017 06:20:27 +
Gerrit-HasComments: Yes

57 matches

Mail list logo