tarun11Mavani commented on PR #13792: URL: https://github.com/apache/pinot/pull/13792#issuecomment-2285714401
> > In #9605, we introduced a change to not resolve non-aggregation group by queries into distinct query, if select clause is not exactly matching to group-by clause. This is a breaking change for our customer as existing query stopped working for us. > > To fix this, instead of throwing an exception, we can return original query and let pinot handle the query as group-by query. > > I think I still I don't understand why this is needed. I understand PR 9605 may have introduced a breaking change. But, isn't that the right thing to do SQL standard wise (In Pinot we have been trying to converge with Postgres) ? If there are no aggregation functions and the SELECT list expressions are not matching the GROUP BY expressions, then it can't be rewritten to DISTINCT and hence throw error. > > > To fix this, instead of throwing an exception, we can return original query and let pinot handle the query as group-by query. > > In your proposal, it is not intuitive to reason about results. What expressions are we going to project as part of user query response ? > > > I think this is standard SQL/Postgres compliant. I tried below queries in mysql and it works fine. > > What about Postgres ? Postgres shows similar behavior when the SELECT clause and the GROUP BY clause do not match, but all columns in the SELECT clause are functionally dependent on the columns in the GROUP BY clause. In such cases, the query executes successfully, projecting only the expressions specified in the SELECT clause. For example, `SELECT row_num FROM testing_groupby GROUP BY row_num, cnt;` or `SELECT length(id) FROM testing_groupby GROUP BY id, nm` are valid queries but `SELECT row_num, cnt FROM testing_groupby GROUP BY row_num;` is not. With this patch pinot will support the first two valid queries while the third query will fail due to the validation check in [validateGroupByClause](https://github.com/apache/pinot/blob/53fbf88027c47b3c6f7d1526576ba0bd257fe9d5/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java#L181). I tried few more queries in Postgres [here](https://onecompiler.com/postgresql/42p26k8uh). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
