gortiz opened a new pull request, #20091: URL: https://github.com/apache/pinot/pull/20091
## Summary This PR adds an option that makes the multi-stage engine resolve a `GROUP BY` name to a `SELECT` alias before a column of the same name. This was the behavior before the Calcite 1.42 upgrade. The option is off by default. The PR also adds a broker meter that counts the queries that the upgrade affects. - Broker config: `pinot.broker.multistage.prefer.select.alias.in.group.by` (default `false`). - Query option: `preferSelectAliasInGroupBy`. It overrides the broker config. - Broker meter: `QUERIES_WITH_GROUP_BY_ALIAS_SHADOWED_BY_COLUMN`. ## Background #18658 upgraded Calcite from 1.40 to 1.42. Calcite 1.42 includes [CALCITE-7052](https://issues.apache.org/jira/browse/CALCITE-7052). That change resolves a simple `GROUP BY` name to a column of the `FROM` clause first. It uses a `SELECT` alias only when no column, or more than one column, has that name. This is the MySQL and PostgreSQL behavior. Calcite 1.40 used the alias first. The same query gives a different plan on each version: ```sql SELECT CASE WHEN col1 = 'foo' THEN 'bar' ELSE col1 END AS col1, SUM(int_col) FROM tbl GROUP BY col1 ``` | Rows `(col1, int_col)` | Calcite 1.40 groups by the `CASE` | Calcite 1.42 groups by the column `col1` | |---|---|---| | `(foo, 1) (foo, 2) (bar, 4) (baz, 8)` | `[bar, 7] [baz, 8]` | `[bar, 3] [bar, 4] [baz, 8]` | When the alias expression uses a column that is not in the `GROUP BY`, the query now fails with `Expression '...' is not being grouped`. When it uses only grouped columns, the query returns different groups with no error. Both cases break queries that worked before the upgrade. ## How `Validator` overrides `performUnconditionalRewrites`, which runs before validation: 1. For each `SqlSelect`, it finds the top-level `GROUP BY` names that match exactly one `SELECT` alias. It skips an alias whose expression is the column of the same name (`col1 AS col1`). 2. If the option is set, it replaces each of these names with the ordinal of its `SELECT` item (`GROUP BY 1`). Calcite always resolves an ordinal to the `SELECT` item. As a result, `HAVING` also resolves the name to the alias, as in Calcite 1.40. `Validator` also overrides `validateGroupClause`. If one of these names is also exactly one column of the `FROM` clause, it marks the meter once for the query. The meter is marked with or without the option. `validateGroupClause` runs before the validation of the `SELECT` list, so the meter also counts the queries that then fail. Operators can use the meter to find affected workloads before they upgrade, or before they turn the option off. Limits: - Names inside `ROLLUP`, `CUBE` and `GROUPING SETS` keep the Calcite 1.42 resolution. - A name that matches more than one alias keeps the Calcite 1.42 resolution. Calcite 1.40 rejected it as ambiguous. - The controller `/sql` endpoint does not read the broker config. The query option works everywhere. - The single-stage engine does not change. ## Tests - `GroupByPlans.json`: plans with and without the option, the `is not being grouped` failure, `HAVING`, and `col1 AS col1`. - `Aggregates.json`: query results with and without the option. These include the case that returns different groups with no error. - `GroupByAliasShadowedByColumnMeterTest`: when the meter is marked and when it is not. This includes failed queries, subqueries, `ORDER BY`, and two affected names in one query. - `ResourceBasedQueryPlansTest`, `QueryCompilationTest`, `ResourceBasedQueriesTest` and `YammerBrokerPrometheusMetricsTest` pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
