somandal commented on code in PR #11577: URL: https://github.com/apache/pinot/pull/11577#discussion_r1326228139
########## pinot-query-planner/src/test/resources/queries/WindowFunctionPlans.json: ########## @@ -1238,6 +1238,19 @@ "\n" ] }, + { Review Comment: OVER() or OVER() with only PARTITION BY both only take default frame as "ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING/CURRENT ROW". Calcite throws an exception if you modify this to be of RANGE type saying an ORDER BY clause must be present. Calcite gets a bit odd here though, even though it accepts "ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING/CURRENT ROW", if you open a debugger and check what values we get for the frame clauses you'll see that _isRows is overridden to FALSE by Calcite and the frame boundaries are always set to UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING, irrespective of if you passed CURRENT ROW for the upper limit or not. So I guess you can say Calcite does implicit RANGE conversion and doesn't even allow a difference in behavior between an upper bound of UNBOUNDED FOLLOWING vs CURRENT ROW if an ORDER BY is not present within the OVER() clause. Interestingly i had checked PostgreSQL behavior prior to starting this PR and in PostgreSQL they do allow OVER() without ORDER BY to use different upper bounds with ROWS and treats the output differently in that case (will do a rolling aggregation for CURRENT ROW). -- 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