Sum, max, min null value fix on non numeric column

[gangadhargo]

Hi @mathiasrw

I tried to fix the following issues please verify and suggest if any changes are required.

#1533
#1532
#1531

[mathiasrw]

Thank you!

[mathiasrw]

I added a few more details to the test. Unfortunately its not unfolding as expected.

[gangadhargo]

Thanks, sure I am reverifying.

[gangadhargo]

Hi @mathiasrw - Sorry, I need some help understanding the code. as i am new to the code base. I tried debugging and the below query.groupfn gives output.
In dojoin.js
query.groupfn(scope, query.params, alasql);
how is the above line resolving the output? where is the actual code which compares the input and gives the result? I want to keep validation there. And keeping validations after output doesn't make sense which I did in my initial commit.

let's say we are doing SUM where is the given input values of sum happening in the code.

[gangadhargo]

Hi @mathiasrw,

I am able to figure out the code and fix the issue. Please ignore the above comment

Thanks,
Gangadhar.

[mathiasrw]

@gangadhargo So happy to hear that. Sorry that my response time has been so long.

[gangadhargo]

@mathiasrw No worries. I have almost fixed it. only one regression test failed. I am debugging it.

[mathiasrw]

Very impressive

[mathiasrw]

Released as part of v2.2.0

[unpete]

alasql('select sum(v) from ?', [[{v: 0}]]) returns [{v: null}] expected [{v: 0}]

[mathiasrw]

@unpete Thank you for this input!

Its very critical.

@gangadhargo Are you able to take a look at this right away? If not I might need to roll the change back and release again. This will break lots of things.

Ill add this to the test in your branch

[mathiasrw]

I made a PR to your branch @gangadhargo gangadhargo#1 pull it in, and make a new PR to develop for this

[mathiasrw]

Reverting as this has created a breaking change. Lest work together in #1553 to solve this.

The reverted version is now out as v2.2.1

[gangadhargo]

sorry guys it was missed. Thanks, @unpete for pointing out this case. @mathiasrw I will work on this

[mathiasrw]

Thank you @gangadhargo

Please start by merging gangadhargo#1 into your repo - and then make a new PR to this branch from yours referring to fixing #1553