vishalmore90 opened a new pull request, #41049:
URL: https://github.com/apache/beam/pull/41049
<!--
Thank you for your contribution! Follow this checklist to help us
incorporate your contribution quickly and easily:
- [ ] If this contribution is large, please file an Apache [Individual
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
- [x] Replace these instructions with a description of your change and why
you have made it.
- [x] Mention the appropriate issue in your description (for example:
`addresses #123` or `fixes #456`).
- [x] Update `CHANGES.md` with noteworthy changes.
See [CONTRIBUTING.md]() for more details.
-->
fixes #41047
### Description
In Beam YAML, global aggregations across an entire PCollection can be
specified without grouping keys (e.g. `group_by: []` or omitting `group_by`).
Previously:
1. When using the Calcite / SQL provider (`Combine-sql`, `Combine-calcite`,
or `language: sql`), `_SqlCombineTransform` unconditionally appended `GROUP BY
%s` formatted with `', '.join(group_by)`. When `group_by` was empty (`[]`),
this generated an invalid SQL query string ending with a dangling `GROUP BY `:
```sql
SELECT sum(c) as total_c FROM PCOLLECTION GROUP BY
```
which Calcite rejected with a syntax parse error (`SqlParseException:
Encountered "<EOF>"`).
2. When `group_by` was omitted entirely from the YAML `config`,
`normalize_combine` did not default `group_by` to an empty list, causing
`set(group_by)` in `_SqlCombineTransform` to raise `TypeError: 'NoneType'
object is not iterable` and `PyJsYamlCombine.__init__` to fail on missing
positional argument `group_by`.
This PR fixes both issues:
- `_SqlCombineTransform`: Defaults `group_by` to `()` and only appends the
`GROUP BY` clause if grouping columns are present.
- `normalize_combine`: Defaults missing/falsy `group_by` configurations to
`[]`.
- `PyJsYamlCombine`: Defaults `group_by` to `()` and `combine` to `{}`.
- Added unit tests in `yaml_combine_test.py` verifying SQL query generation
with grouping keys, with `group_by: []`, with omitted `group_by`, and YAML
execution without `group_by`.
- Updated `CHANGES.md`.
--
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]
