Opened 7 years ago
Closed 7 years ago
#1596 closed defect (fixed)
WCPS1.5_Remove the check for applying trimming/slicing in dimensions interval
Reported by: | Bang Pham Huu | Owned by: | Bang Pham Huu |
---|---|---|---|
Priority: | major | Milestone: | 9.5 |
Component: | petascope | Version: | development |
Keywords: | Cc: | Dimitar Misev, Vlad Merticariu | |
Complexity: | Medium |
Description
In SubsetExpressionHandler class, Vlad found that the else condition here is not correct and all cases should be handled by the first condition.
// NOTE: in case of coverage constant e.g: <[-1:1,-1:1],-1,0,1,....> it should not replace the interval inside the "< >" if (rasql.contains("<") || !rasql.contains("[")) { String dimensions = rasqlTranslationService.constructRasqlDomain(metadata.getSortedAxesByGridOrder(), axisIteratorSubsetDimensions, axisIteratorAliasRegistry); rasqlSubset = template.replace("$dimensionIntervalList", dimensions); } else { // update the interval of the existing expression in template string // e.g: trim(c[0:5,0:100,89], {90:90}) -> c[0:5,90:90,89] // need to replace the interval correctly String tmp = rasql.substring(rasql.indexOf("[") + 1, rasql.indexOf("]")); String[] intervals = tmp.split(","); String axisName = subsetDimensions.get(0).getAxisName(); Axis axis = metadata.getAxisByName(axisName); int axisOrder = axis.getRasdamanOrder(); String dimension = rasqlTranslationService.constructRasqlDomain(ListUtil.valuesToList(axis), axisIteratorSubsetDimensions, axisIteratorAliasRegistry); intervals[axisOrder] = dimension; // 0:5,0:100,89 String intervalsStr = "[" + StringUtils.join(intervals, ",") + "]"; rasqlSubset = rasql.replaceAll("\\[.*?\\]", intervalsStr); }
So if the tests for WCPS have no problem then can remove the if else condition.
Change History (2)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Note:
See TracTickets
for help on using tickets.
Just to give a little more context:
The else branch handles the cases of nested subsets, e.g.
(c[Lat(0:10), Long(0:10)])[Lat(0), Long(0)] will be translated into rasql in the same way as c[Lat(0), Long(0)], resulting in a single interval.
The problem is that it is assumed that the second subset is applied on the same metadata as the first one, basically overriding the first subset . This is not always the case, there can be an operation that changes the metadata:
avg(c[Lat(0:10), Long(0:10)])[Lat(0), Long(0)] will move the subset inside the avg, and will be translated to avg(c[Lat(0), Long(0)]) when in fact the outer subset is applicable to a different object, and should result in an error.
Since rasql supports nested subsets, I think we shouldn't attempt to rewrite the wcps query, and simply forward it to rasql with 2 subsets instead of merging them heuristically. This would be achieved by removing the if and keeping the first branch only.