Addition of Freq distribution

[PRijnbeek]

Creates the results needed for a frequency distribution of concepts.

It calculates the number of people that have more than 1,2,3,4... of the concept, e.g. >1 BMI.

In Atlas and WebApi the functionality is added for the plot creation which will be pulled separately.

See feature in WebAPI: link

[anthonysena]

Per #208, this is 1 of 2 features related to the V2.2.0 release of Atlas/WebAPI (OHDSI/WebAPI#217).

I wanted to raise a procedural point for this pull request and #212. I'd recommend that this PR be reviewed first since it is contributing new analyses and once it is reviewed and approved, it could be merged into master. At that point, I think it would make sense to call this v1.5.0 of Achilles.

PR #212 adds important performance enhancements for the use of Achilles data in WebAPI/Atlas. There are new enhancements for Atlas/WebAPI V2.2.0 that depend on these changes. As a result, once #212 is reviewed, I think it will be good to mark this as a v1.6.0 so that the v2.2.0 can reference it.

Please let me know if you have any concerns or alternative suggestions. I'll also post something similar to #212 for reference.

[t-abdul-basser]

• Agreed re desirability of reviewing of this PF first. I am reviewing and will include any comments in review thread in case @chrisknoll or @vojtechhuser wan tot join review.

• Milestone v1.5.0 created added to this PR.

[anthonysena]

FYI - @t-abdul-basser and others - I'm doing a review of this PR on my side in order to ensure that it works in our environment and will likely have some changes for @PRijnbeek to review and commit to this branch.

[t-abdul-basser]

Thanx that is fine. Need a bit more time on my end.

[anthonysena]

@t-abdul-basser just bumping this PR to see if we can get another review and hopefully get this merged? As a note, I've reviewed and tested this code in our environment and everything is working well.

[vojtechhuser]

I don't see the option to formally review but the changes are adding measures and SQL code. If the TravisCI testing shows no errors I see no reason not to merge.

[t-abdul-basser]

Sorry for the delay. Reviewed. No apparent issue.

[chrisknoll]

The error reported in the Travis CI Log was related to oracle:

Error:
java.sql.SQLSyntaxErrorException: ORA-00933: SQL command not properly ended
SQL:
insert into [secure].ACHILLES_results (analysis_id, stratum_1, stratum_2, count_value)
SELECT 691 as analysis_id,
	procedure_concept_id as stratum_1, 
	prc_cnt as stratum_2,
	sum(count(person_id))	over (partition by procedure_concept_id order by prc_cnt desc) as count_value
 FROM (SELECT p.procedure_concept_id, 
		count(p.procedure_occurrence_id) as prc_cnt, 
		p.person_id
	FROM [secure].procedure_occurrence p 
	group by p.person_id, p.procedure_concept_id
 ) as cnt_q
group by procedure_concept_id, prc_cnt 

I see information here: https://www.tekstream.com/resources/ora-00933-sql-command-not-properly-ended/ that talks about ORDER BYs being used in inserts and the only place I can see that happening is in the SUM (count(person_id)) over (partition by...order by...). I'm wondering if the order by is necessary in this case and causing the problem on oracle?

[chrisknoll]

Nevermind, I see how the order by is trying to create a running total by ordering based on prc_cnt. So I'm not sure why Oracle is having a problem with this query...I'd need an actual oracle instance to try it.

@t-abdul-basser : do you have access to an Oracle instance to try to run this query and see what the issue is?

[chrisknoll]

Further research: found this SO post: https://stackoverflow.com/questions/9811711/sql-command-not-properly-ended

It indicates that you don't use AS {alias} for subqueries, just put the Alias, so I think the fix is to remove 'as' from the subquery.

FROM (...) cnt_q

[t-abdul-basser]

Thanks @anthonysena. Perhaps we can ask @PRijnbeek @lhalvors to make this change and submit a new PR (since I already merged the other).