Okay. That sounds good as well. On May 16, 2017 12:47 AM, "Alexander Behm" <[email protected]> wrote:
> Vincent, after deliberating with Greg Rahn, we concluded the second option > (rewrite) would be preferable. Sorry to go back and forth on this. > > Reasons: > - We will likely soon add more functions that can make use of the same > "accept in analysis + rewrite later" mechanism > - The rewrite solution avoids code duplication > - We can avoid generating many useless functions that just served the > purpose of accepting multiple first arg types > > Please let me know if you need guidance to make progress. This rewrite > solution might be more complicated to implement. > > Alex > > On Mon, May 15, 2017 at 8:19 PM, Vincent Tran <[email protected]> wrote: > > > That did the job. Thanks Bharath. > > > > On Mon, May 15, 2017 at 11:09 PM, Bharath Vissapragada < > > [email protected]> wrote: > > > > > Added you to the relevant groups, could you check again please? > > > > > > On Mon, May 15, 2017 at 7:57 PM, Vincent Tran <[email protected]> > > wrote: > > > > > > > My handle on ASF Jira is thundergun. > > > > > > > > On Mon, May 15, 2017 at 10:20 PM, Tim Armstrong < > > [email protected] > > > > > > > > wrote: > > > > > > > > > Not sure - if you tell us your username on Apache JIRA we could > just > > > > assign > > > > > it to you. > > > > > > > > > > On Mon, May 15, 2017 at 6:13 PM, Vincent Tran <[email protected] > > > > > > wrote: > > > > > > > > > > > Thanks for the tips. I will go with option 3. > > > > > > > > > > > > Also, for some reasons, I don't have permissions to assign issues > > to > > > > > myself > > > > > > in the Impala project. Do you guys know if I need to request > > > additional > > > > > > privileges? > > > > > > > > > > > > I can assign Hive issues to myself without additional actions. > > > > > > > > > > > > > > > > > > Vincent > > > > > > > > > > > > > > > > > > On Mon, May 15, 2017 at 6:55 PM, Tim Armstrong < > > > > [email protected]> > > > > > > wrote: > > > > > > > > > > > > > Also if you're starting to work on the JIRA, can assign it to > > > > yourself > > > > > in > > > > > > > the Apache JIRA instance so we can see that it's assigned to > > > someone. > > > > > > > > > > > > > > Cheers, > > > > > > > Tim > > > > > > > > > > > > > > On Mon, May 15, 2017 at 4:21 PM, Alexander Behm < > > > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > Hi Vincent, > > > > > > > > > > > > > > > > Jim is correct, we to be able to handle invocations with > those > > > > types: > > > > > > > > The 1st arg can be any type. The 2nd and 3rd types must be > > > > > compatible. > > > > > > > > During function resolution the FE will pick the most > > appropriate > > > > > > > signature > > > > > > > > and add casts to the 2nd or 3rd arg as necessary. > > > > > > > > You do not need to create signatures like "nvl2(string, int, > > > > > > timestamp)". > > > > > > > > > > > > > > > > I have considered the following options for addressing this > > > > > difficulty: > > > > > > > > 1. Accept an AnyType for the 1st argument and create 9 > function > > > > > > > signatures. > > > > > > > > The FE current does not have such a concept. Adding it would > > > > involve > > > > > > > > changing many tricky places. > > > > > > > > 2. Rewrite NVL2() as an IF in the FE using the > ExprRewriteRule > > > > > > framework. > > > > > > > > You will need to make changes to FunctionCallExpr to accept > > > NVL2(). > > > > > > > > 3. Stamp out all 9*9 function signatures. Should be easy > since > > > you > > > > > are > > > > > > in > > > > > > > > Python code there. > > > > > > > > > > > > > > > > Option 3 seems easiest/preferable to me at this point. > > > > > > > > > > > > > > > > Alex > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, May 15, 2017 at 1:05 PM, Jim Apple < > > [email protected] > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > It seems to me like there should be version with type X, > > > [P,X,X] > > > > > for > > > > > > > > > all pair P and X. There might be a smart way to do this > > without > > > > the > > > > > > > > > quadratic number of cases; someone else probably knows > better > > > > than > > > > > I > > > > > > > > > do about that. > > > > > > > > > > > > > > > > > > On Sun, May 14, 2017 at 1:00 AM, Vincent Tran < > > > > [email protected] > > > > > > > > > > > > > > wrote: > > > > > > > > > > Hey folks, > > > > > > > > > > > > > > > > > > > > I want to start contributing to learn the code base. I > > added > > > > this > > > > > > > > > function > > > > > > > > > > to my personal build: > > > > > > > > > > > > > > > > > > > > [['nvl2'], 'TINYINT', ['TINYINT', 'TINYINT', > 'TINYINT'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'SMALLINT', ['SMALLINT', 'SMALLINT', > > > 'SMALLINT'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'INT', ['INT', 'INT', 'INT'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'BIGINT', ['BIGINT', 'BIGINT', 'BIGINT'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'FLOAT', ['FLOAT', 'FLOAT', 'FLOAT'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'DOUBLE', ['DOUBLE', 'DOUBLE', 'DOUBLE'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'DECIMAL', ['DECIMAL', 'DECIMAL', > 'DECIMAL'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'STRING', ['STRING', 'STRING', 'STRING'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > [['nvl2'], 'TIMESTAMP', ['TIMESTAMP', 'TIMESTAMP', > > > > > 'TIMESTAMP'], > > > > > > > > > > 'impala::ConditionalFunctions::NVL2'], > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you think this is a sound approach? Should we allow > mix > > > > types > > > > > > for > > > > > > > > this > > > > > > > > > > function? > > > > > > > > > > > > > > > > > > > > i.e. nvl2(string, int, timestamp) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://issues.cloudera.org/browse/IMPALA-5030 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > Vincent T. Tran > > > > > > > > > > Customer Operations Engineer > > > > > > > > > > Cloudera, Inc. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Vincent T. Tran > > > > > > Customer Operations Engineer > > > > > > Cloudera, Inc. > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Vincent T. Tran > > > > Customer Operations Engineer > > > > Cloudera, Inc. > > > > > > > > > > > > > > > -- > > Vincent T. Tran > > Customer Operations Engineer > > Cloudera, Inc. > > >
