hpcc-systems / DataPatterns

HPCC Systems ECL bundle that provides some basic data profiling and research tools to an ECL programmer
3 stars 4 forks source link

"rec_cnt" for all possible distributions will need to support numbers greater than what unsigned4 would allow #81

Open ManjunathVenkataswamy opened 4 months ago

ManjunathVenkataswamy commented 4 months ago

The 'rec_cnt' that comes out in the response might need to be changed from being a unsigned4 to unsigned. We have noticed it created an issue with a pattern count that was greater than what a unsgined4 could hold. And down the lane, it ended up coming out as a junk number that made no sense. Here is an example of how this would cause issues that depend on datapattern library for profiling the data. if you run a code like this: unsigned4 test := 4449418834; test; you would get 154451538 !!!

Discussed this need with Dan and creating this ticket.

GordonSmith commented 4 months ago

@dcamper need to sanity check the viz when going from 32bit -> 64bit numbers (JS doesn't support them and they end up being strings)

dcamper commented 4 months ago

@GordonSmith good point. What would be the best workaround for values exceeding 2^32? (Or is it 2^31?)

GordonSmith commented 4 months ago

Its actually converted to a decimal with precision 2^56 (from memory), so you start to lose the insignificant values - but - it gets sent as a string which sometimes catches us out.

dcamper commented 4 months ago

@ManjunathVenkataswamy: Would an UNSIGNED6 suffice for your needs? That would give you a maximum value of 281,474,976,710,655 (2^48 - 1) and yet comfortably remain a number for visualization purposes.

ManjunathVenkataswamy commented 4 months ago

yes it would.

dcamper commented 4 months ago

@ManjunathVenkataswamy: If you have the ability, please check out the candidate-1.10.0 branch (https://github.com/hpcc-systems/DataPatterns/tree/candidate-1.10.0) and test it. Once accepted, I can get it merged into the platform's Std ECL library.

ManjunathVenkataswamy commented 3 months ago

I haven't yet been able to test this. But will do so within 2 days.