OpenRefine: pow() does not throw error for extra or incorrect arguments like quotient() does

To Reproduce

Steps to reproduce the behavior:

Open Expression Editor on existing project
On any column use Custom Text Transform with value.pow(2,8)
Notice the preview result of null
Change the expression to value.quotient(2,8)
Notice the preview result of Error: quotient expects two numbers

Current Results

null is shown in preview result, instead of an error (since value is passed as one of the arguments with additional 2 arguments (2,8) supplied)

Expected Behavior

pow() should check arguments like quotient() does and show similar error.

Screenshots

Versions

Operating System: Windows 10
Browser Version: Firefox latest
JRE or JDK Version: JDK8
OpenRefine: openrefine-3.4-beta-458-g109aa78

Datasets

Additional context

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 18 (18 by maintainers)

Most upvoted comments

@elebitzero we can definitely work together so this can be resolved 😃😁 I’ll go through your PR and select out what is not included so I can launch my help too.

elroykanye on Apr 8, 2022

@antoine2711 @elroykanye, oops, I’m just seeing these recent comments now. It looks like I picked up this issue at the same time @elroykanye started looking into it. I should have commented on the issue first.

I fixed the issue where null.pow(2) returns null instead of an error like all the other math functions. Also, I found that null.exp() returned null instead of an error. So the issue fixed by my PR is that pow() and exp() now return Error instead of null for null arguments, so they are now consistent with the other math functions.

I also added a new test testNullArgsMath to check that the math functions return an error with null argument(s).

Note: @thadguidry mentioned value.pow(2,8) which is not valid because pow() expects two numbers. In this case, my fix will display an error instead of null.

elebitzero on Apr 7, 2022

Alright then, I will go through each and enforce the pattern of returning an EvalError if there are no objections.

elroykanye on Apr 3, 2022

What do you intend to compute with value.pow(2,8)? If you want to compute 2*2*2*2*2*2*2*2 (aka 256), then you should just use pow(2,8), not value.pow(2, 8). Same for quotient.

Yes, it’s an error, but I think the point of the original bug report was that the error handling for the two cases should be equivalent. They should either both return error or both return null. I have a strong preference for the former.

This isn’t limited to pow(). All of these functions are exempt from the test for too many arguments:

https://github.com/OpenRefine/OpenRefine/blob/c8a721a10bbdba092b3605204aa9849da3089e5f/main/tests/server/src/com/google/refine/grel/FunctionTests.java#L133-L137

"chomp", "contains", "coalesce", "escape", "unescape", "exp", "fingerprint", "get", "now", "parseJson", "partition", "pow", "rpartition", "slice", "substring", "unicode", "unicodeType"

tfmorris on Oct 27, 2020

@tfmorris or @wetneb will check (they are 2 of our core developers) as I’m staying out of active development. But I will be happy to test once merged.

thadguidry on Aug 21, 2020