maytasm commented on pull request #9986: URL: https://github.com/apache/druid/pull/9986#issuecomment-639102247
@gianm Thank you for your review. Initially, I thought that was a valid case where we can have positions in aggregate.getGroupSet() not matching fields in the rowSignature. But as you pointed out I think that is reasonable too and there is a bug in how we determine the rowSignature. You mentioned that rowSignature comes from computeOutputRowSignature(sourceRowSignature, selectProjection, null, null), which is the source data plus any pre-aggregation projections (selectProjection). However, selectProjection in computeOutputRowSignature(sourceRowSignature, selectProjection, null, null) will always be null. So rowSignature that is pass to computeGrouping for computeGrouping is only source data excludes any pre-aggregation projections (selectProjection). I did see that other methods in computeGrouping such as computeDimensions and computeAggregations respect selectProjection by getting the selectProjection from partialDruidQuery. I think computeSubtotals should also respect selectProjection in partialDruidQuery if selectProjection exists. I have modify computeSubtotals to get the fields from selectProjection inside partialDruidQuery and ignore the original rowSignature passed into the method. Getting the fieldCount from selectProjection inside partialDruidQuery is done similar to Projection.preAggregation (in computeSelectProjection), this should make sure that each projection is valid. Please let me know what you think. Thanks! ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
