You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by ryzuo <gi...@git.apache.org> on 2016/11/28 10:28:06 UTC

[GitHub] incubator-trafodion pull request #854: [TRAFODION - 2345] Support LOG() func...

GitHub user ryzuo opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/854

    [TRAFODION - 2345] Support LOG() function of any base

    Like Oracle does, the function LOG() function supports 2 parameter of call, in this way, the 1st parameter is the radix, and the second one is power, the function returns the logarithm.
    This change implement this, and still, the user can call it with only one parameter (form LOG(X)), in which case the parameter X will be the power, and the function just returns the natural logarithm of X. This is also compatible with MySQL.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ryzuo/incubator-trafodion jira2345

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/854.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #854
    
----
commit 1f9071ae82ee63e6f16bddd091d3fcce1057583f
Author: ryzuo <jo...@gmail.com>
Date:   2016-11-25T18:30:26Z

    Implement LOG() function for any base

commit e4681d139f1b3e0ffd7784d9930e9fc0d72bbcbb
Author: ryzuo <jo...@gmail.com>
Date:   2016-11-28T08:39:45Z

    Add tests for builtin function LOG() of any base

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #854: [TRAFODION - 2345] Support LOG() func...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-trafodion/pull/854


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #854: [TRAFODION - 2345] Support LOG() func...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/854#discussion_r89907859
  
    --- Diff: core/sql/exp/exp_math_func.cpp ---
    @@ -337,8 +338,22 @@ ex_expr::exp_return_type ExFunctionMath::eval(char *op_data[],
     	  **diagsArea << DgString0("LOG");
     	  return ex_expr::EXPR_ERROR;
     	}
    -      
    -      *(double *)op_data[0] = MathLog(*(double *)op_data[1], err);
    +
    +      if(3 == getNumOperands())
    +    {
    +        double power = *(double *)op_data[2];
    +        double radix = *(double *)op_data[1];
    +        if(power <= 0 || 1 == radix)
    --- End diff --
    
    The radix also has to be greater than zero, doesn't it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #854: [TRAFODION - 2345] Support LOG() func...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/854#discussion_r89907128
  
    --- Diff: core/sql/exp/exp_math_func.cpp ---
    @@ -337,8 +338,22 @@ ex_expr::exp_return_type ExFunctionMath::eval(char *op_data[],
     	  **diagsArea << DgString0("LOG");
     	  return ex_expr::EXPR_ERROR;
     	}
    -      
    -      *(double *)op_data[0] = MathLog(*(double *)op_data[1], err);
    +
    +      if(3 == getNumOperands())
    +    {
    --- End diff --
    
    The indentation is inconsistent. Perhaps your editor is using tabs? Best to replace those with spaces.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request #854: [TRAFODION - 2345] Support LOG() func...

Posted by ryzuo <gi...@git.apache.org>.
Github user ryzuo commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/854#discussion_r89934382
  
    --- Diff: core/sql/exp/exp_math_func.cpp ---
    @@ -337,8 +338,22 @@ ex_expr::exp_return_type ExFunctionMath::eval(char *op_data[],
     	  **diagsArea << DgString0("LOG");
     	  return ex_expr::EXPR_ERROR;
     	}
    -      
    -      *(double *)op_data[0] = MathLog(*(double *)op_data[1], err);
    +
    +      if(3 == getNumOperands())
    +    {
    +        double power = *(double *)op_data[2];
    +        double radix = *(double *)op_data[1];
    +        if(power <= 0 || 1 == radix)
    --- End diff --
    
    you are right, I will change this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---