Repository navigation
Expand Beam SQL TPC-DS query support and add Python TPC-DS suite - #40695
Conversation
| with: | ||
| persist-credentials: false | ||
| - name: Setup repository | ||
| uses: ./.github/actions/setup-action |
| - name: Setup repository | ||
| uses: ./.github/actions/setup-action | ||
| - name: Setup environment | ||
| uses: ./.github/actions/setup-environment-action |
| python-version: default | ||
| go-version: default | ||
| - name: Run full TPC-DS suite on PrismRunner with regression check | ||
| uses: ./.github/actions/gradle-command-self-hosted-action |
- Add `BeamCrossJoinRel` and `BeamCrossJoinRule` to support bounded-by-bounded pure cross joins in Beam Calcite SQL. - Extend `BeamCoGBKJoinRule` to split mixed equi/non-equi inner join predicates into an equi-join followed by a post-join `BeamCalcRel` filter. - Support `LAG`, `LEAD`, `FIRST_VALUE`, `LAST_VALUE`, `NTH_VALUE` (`RESPECT`/`IGNORE NULLS`), and `CUME_DIST` analytic window functions in `BeamWindowRel` and `BeamBuiltinAnalyticFunctions`, and convert window inputs to `BeamLogicalConvention` in `BeamWindowRule`. - Fix `NULLS FIRST` and `NULLS LAST` comparator ordering in `BeamSortRel` when combined with `ASC`/`DESC`. - Normalize set operator (`UNION`, `INTERSECT`, `EXCEPT`) input schemas in `BeamSetOperatorRelBase` when inputs differ only by field names or nullability. - Return `NULL` instead of `0.0` from `VarianceFn` for empty or single-element sample variance/stddev groups to match SQL standard semantics. - Remove workarounds in TPC-DS table schemas used to cast `DECIMAL` to `DOUBLE` for fractional columns Having these improvements, 55 out of 103 queries now passing (up from 24). Then for test suite, - add `--queries=supported` and `10M`/`10MB` dataset size support to `TpcdsParametersReader`. - Add the Python TPC-DS benchmark and regression test suite (`sdks/python/apache_beam/testing/benchmarks/tpcds`) running on `PrismRunner` and cross-language `SqlTransform`. - Update Java TPC-DS GitHub Actions workflows to use `--queries=supported` and add the `beam_PostCommit_Python_Tpcds_Prism.yml` workflow. - Move query result file to a temp bucket with lifecycle policy, prevent resource leak
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40695 +/- ##
=============================================
+ Coverage 56.16% 77.42% +21.25%
- Complexity 2288 2359 +71
=============================================
Files 1124 814 -310
Lines 178336 98969 -79367
Branches 1489 2029 +540
=============================================
- Hits 100170 76629 -23541
+ Misses 75645 21573 -54072
+ Partials 2521 767 -1754
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
PreCommit SQL Java17 failing errorprone in HEAD, not related to this change |
|
R: @kennknowles |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
kennknowles
left a comment
There was a problem hiding this comment.
This seems really useful. LGTM modulo a few comments. Hopefully easy to automate clean up.
| @Override | ||
| public BeamCostModel beamComputeSelfCost(RelOptPlanner planner, BeamRelMetadataQuery mq) { | ||
| NodeStats inputStat = BeamSqlRelUtils.getNodeStats(this.input, mq); | ||
| // Use the planner's row-count (RelSubset-safe) rather than BeamSqlRelUtils.getNodeStats, which |
There was a problem hiding this comment.
I wonder if BeamSqlrelUtils.getNodeStats should be changed, if we are replacing it with all these trivial inlines. FWIW this comment is actually an OK explanation for the code.
There was a problem hiding this comment.
acked. This could be a future improvement
BeamCrossJoinRelandBeamCrossJoinRuleto support bounded-by-bounded pure cross joins in Beam Calcite SQL.BeamCoGBKJoinRuleto split mixed equi/non-equi inner join predicates into an equi-join followed by a post-joinBeamCalcRelfilter.LAG,LEAD,FIRST_VALUE,LAST_VALUE,NTH_VALUE(RESPECT/IGNORE NULLS), andCUME_DISTanalytic window functions inBeamWindowRelandBeamBuiltinAnalyticFunctions, and convert window inputs toBeamLogicalConventioninBeamWindowRule.NULLS FIRSTandNULLS LASTcomparator ordering inBeamSortRelwhen combined withASC/DESC.UNION,INTERSECT,EXCEPT) input schemas inBeamSetOperatorRelBasewhen inputs differ only by field names or nullability.NULLinstead of0.0fromVarianceFnfor empty or single-element sample variance/stddev groups to match SQL standard semantics.DECIMALtoDOUBLEfor fractional columnsHaving these improvements, 55 out of 103 queries now passing (up from 24). Then for test suite,
--queries=supportedand10M/10MBdataset size support toTpcdsParametersReader.sdks/python/apache_beam/testing/benchmarks/tpcds) running onPrismRunnerand cross-languageSqlTransform.--queries=supportedand add thebeam_PostCommit_Python_Tpcds_Prism.ymlworkflow.