somandal commented on PR #9078: URL: https://github.com/apache/pinot/pull/9078#issuecomment-1191941342
> +1 on both suggestions. > > from this claim: > > > The position of the MV column has no affect on the actual ordering of results as the SelectionOrderByOperator only includes the SV columns in the comparator used to set the values in the PriorityQueue maintained by it. The query execution failure only occurs when the MV column is the first expression. > > I think we should definitely not allow MV values present in the order-by list with explicit error, and we can do that in the parser level already right? I did consider that, but we may need to add more checks on the broker level if we wanted to do it there. The server side figured out which queries are aggregation types already and creates a different PlanNode for that. We'd have to add some of that logic on broker side if we wanted to do this at the parser level. Also, this is not a problem if GROUP BY is included in the query as GROUP BY flattens out the column. Let me know your thoughts. `QueryContextUtils.isAggregationQuery(queryContext)` -> in `makeSegmentPlanNode()` is pre-populated when the QueryContext is created. -- 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. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
