Check for non-numerical values in numerical data count filter

[onursumer]

Fix #11204

This only fixes numericalClinicalDataCountFilter.

Now applied the same fix to numericalGenomicDataFilter and numericalGenericAssayDataFilter as well by unifying the filter logic under one SQL.

TODO: add unit tests with variety of clinical, genomic, and generic assay data filter combinations

[fuzhaoyuan]

Maybe we don't need to handle NA here if this SQL is used as the non-NA part? Just curious, it's also fine to have it here. And I think it's also possible to include the NA part as a whole SQL but this is great progress already. Appreciate it.

[onursumer]

I think this NA part was implemented before we added the additional logic for NA.

You're right. We should probably move the NA filter logic into this same SQL as well. Might be better to deal with it in a separate PR tough.

[fuzhaoyuan]

lgtm

[haynescd]

I wouldn't do the normalizeAttributeValue. You are just testing multiple values of NA?

[onursumer]

normalizeAttributeValue used to normalize values other than NA but we got rid of that part at some point by upper casing everything. So I guess normalizing here is redundant now.

We are still doing this in multiple places where we filter categorical data. For example, see

cbioportal/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml

Lines 483 to 489 in d9459f3

	<otherwise>
	(
	<include refid="normalizeAttributeValue">
	<property name="attribute_value" value="attribute_value"/>
	</include>
	) ILIKE #{dataFilterValue.value}
	</otherwise>

and

cbioportal/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewFilterMapper.xml

Lines 614 to 620 in d9459f3

	<otherwise>
	(
	<include refid="normalizeAttributeValue">
	<property name="attribute_value" value="value"/>
	</include>
	) ILIKE #{dataFilterValue.value}
	</otherwise>

These are also most likely redundant now, and I think we should address this duplication as well, but probably in a separate PR.

[onursumer]

Removed normalizeAttributeValue from all these 3 places. We may still want to address the code duplication for categorical filters at some point.

[haynescd]

Looks good. Just had one question

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud