Return-Path: X-Original-To: apmail-hive-dev-archive@www.apache.org Delivered-To: apmail-hive-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 50A1110D3B for ; Tue, 26 Nov 2013 17:27:42 +0000 (UTC) Received: (qmail 53650 invoked by uid 500); 26 Nov 2013 17:27:25 -0000 Delivered-To: apmail-hive-dev-archive@hive.apache.org Received: (qmail 53573 invoked by uid 500); 26 Nov 2013 17:27:21 -0000 Mailing-List: contact dev-help@hive.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hive.apache.org Delivered-To: mailing list dev@hive.apache.org Received: (qmail 53506 invoked by uid 99); 26 Nov 2013 17:27:04 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 26 Nov 2013 17:27:04 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 447CB1D3C12; Tue, 26 Nov 2013 17:27:00 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============2768513019676053357==" MIME-Version: 1.0 Subject: Re: Review Request 15777: HIVE-5706: Move a few numeric UDFs to generic implementations From: "Brock Noland" To: "Xuefu Zhang" , "hive" , "Brock Noland" Date: Tue, 26 Nov 2013 17:27:00 -0000 Message-ID: <20131126172700.5111.48222@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Brock Noland" X-ReviewGroup: hive X-ReviewRequest-URL: https://reviews.apache.org/r/15777/ X-Sender: "Brock Noland" References: <20131125232748.32080.38292@reviews.apache.org> In-Reply-To: <20131125232748.32080.38292@reviews.apache.org> Reply-To: "Brock Noland" X-ReviewRequest-Repository: hive-git --===============2768513019676053357== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Nov. 25, 2013, 11:27 p.m., Xuefu Zhang wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java, line 85 > > > > > > Runtime exceptions represent problems that are the result of a programming problem, which seems proper for this case. That's, unless I programmed wrong, it shouldn't happen, which is exactly the case. > > > > I'm not sure if other type exception really buy us anything. IllegalStateException provides better semantics. That is we are saying "an illegal state occurred" simply by the name which is more specific than simply "some kind of runtime error occured". I understand this is a small point but if there is a better semantic name for an exception, we should use it. - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15777/#review29406 ----------------------------------------------------------- On Nov. 22, 2013, 5:52 a.m., Xuefu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15777/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2013, 5:52 a.m.) > > > Review request for hive. > > > Bugs: HIVE-5706 > https://issues.apache.org/jira/browse/HIVE-5706 > > > Repository: hive-git > > > Description > ------- > > 1. Replaced the old implementations of power, ceil, floor, positive, and negative with generic UDF implmentations. > 2. Added unit tests for each. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 435d6e6 > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 0a79256 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 151c648 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFCeil.java a01122e > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFFloor.java 3fdaf88 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPNegative.java bab1105 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPPositive.java ae11d74 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFPower.java 184c5d2 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseUnary.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCeil.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFloor.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFFloorCeilBase.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPNegative.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPositive.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFPower.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorizationContext.java 73bcee0 > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFCeil.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFFloor.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPNegative.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPPositive.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFPower.java PRE-CREATION > ql/src/test/results/clientpositive/decimal_udf.q.out ed5bc65 > ql/src/test/results/clientpositive/literal_decimal.q.out 78dac31 > ql/src/test/results/clientpositive/udf4.q.out 50db96c > ql/src/test/results/clientpositive/udf7.q.out 7316449 > ql/src/test/results/clientpositive/vectorization_short_regress.q.out c9296e1 > ql/src/test/results/clientpositive/vectorized_math_funcs.q.out 8bb0edf > ql/src/test/results/compiler/plan/udf4.q.xml 145e244 > > Diff: https://reviews.apache.org/r/15777/diff/ > > > Testing > ------- > > All new tests and old test passed. > > > Thanks, > > Xuefu Zhang > > --===============2768513019676053357==--