Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds the COLL_* functions (COLL_AVG, COLL_COUNT, COLL_MAX, COLL_MIN, COLL_SUM) #353

Merged
merged 1 commit into from
May 25, 2023

Conversation

alancai98
Copy link
Member

Implements the COLL_ functions (COLL_AVG, COLL_COUNT, COLL_MAX, COLL_MIN, COLL_SUM). Also

  • Fixes slight bug in the aggregation function regex
  • Upgrades lalrpop to latest version to get rid of warnings

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@alancai98 alancai98 self-assigned this May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Patch coverage: 84.16% and project coverage change: +0.02 🎉

Comparison is base (425d3cc) 81.44% compared to head (cb9dbbd) 81.46%.

❗ Current head cb9dbbd differs from pull request most recent head 49d84c4. Consider uploading reports for the commit 49d84c4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #353      +/-   ##
==========================================
+ Coverage   81.44%   81.46%   +0.02%     
==========================================
  Files          51       44       -7     
  Lines       11579    10903     -676     
==========================================
- Hits         9430     8882     -548     
+ Misses       2149     2021     -128     
Impacted Files Coverage Δ
partiql-eval/src/eval/evaluable.rs 84.17% <0.00%> (-0.13%) ⬇️
partiql-logical/src/lib.rs 45.31% <0.00%> (-0.72%) ⬇️
partiql-parser/src/parse/parser_state.rs 96.55% <ø> (ø)
partiql-eval/src/eval/expr/mod.rs 81.47% <82.26%> (+0.46%) ⬆️
partiql-logical-planner/src/call_defs.rs 85.37% <85.71%> (ø)
partiql-eval/src/plan.rs 96.16% <100.00%> (+<0.01%) ⬆️

... and 23 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link

github-actions bot commented May 5, 2023

Conformance comparison report

Base (425d3cc) 3b09201 +/-
% Passing 80.48% 82.43% 1.95%
✅ Passing 2766 2833 67
❌ Failing 477 410 -67
🔶 Ignored 194 194 0
Total Tests 3437 3437 0

Number passing in both: 2766

Number failing in both: 410

Number passing in Base (425d3cc) but now fail: 0

Number failing in Base (425d3cc) but now pass: 67

The following test(s) were previously failing but now pass. Before merging, confirm they are intended to pass:

Click here to see
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_list_of_missing_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_missing
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_top_level_agg_coll_avg_all_data_result_success_1_25
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::top_level_distinct_coll_avg
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::top_level_coll_min
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_null
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_missing
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::top_level_distinct_coll_sum
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_top_level_agg_coll_avg_distinct_data_result_success_1_5
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::top_level_all_coll_count
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_null
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_empty_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_top_level_agg_coll_sum_all_data_result_success_5
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_top_level_agg_coll_sum_distinct_data_result_success_3
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_list_of_missing_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_non_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_list_of_missing_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_top_level_agg_coll_count_data_result_success_4
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_mistyped_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_null
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_empty_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_bag_of_missing_elements
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_empty_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_non_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::top_level_distinct_coll_max
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::top_level_all_coll_sum
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::top_level_coll_avg_only_int
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_empty_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_bag_of_heterogeneous_element_types
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::top_level_all_coll_min
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_top_level_agg_coll_max_distinct_data_result_success_2
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_top_level_agg_coll_min_all_data_result_success_1
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_agg_in_select::select_value_coll_aggregate
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_missing
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_top_level_agg_coll_min_data_result_success_1
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_null
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_null
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::top_level_coll_avg
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::max_top_level_agg_coll_max_data_result_success_2
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_bag_of_heterogeneous_element_types
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::top_level_coll_count_distinct
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_bag_of_missing_elements
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_bag_of_missing_elements
  • partiql_tests::eval::spec_tests::section_11::coll_count_without_group_by
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_top_level_agg_coll_sum_data_result_success_5
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_bag_of_heterogeneous_element_types
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_top_level_agg_coll_count_distinct_data_result_success_2
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::top_level_coll_count
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_list_of_missing_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_bag_of_missing_elements
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_count::coll_count_top_level_agg_coll_count_all_data_result_success_4
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_non_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_mistyped_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::top_level_distinct_coll_min
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_top_level_agg_coll_avg_data_result_success_1_25
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::top_level_coll_max
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_empty_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::top_level_coll_sum
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_sum::coll_sum_missing
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_top_level_agg_coll_max_all_data_result_success_2
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_missing
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_bag_of_missing_elements
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_avg::coll_avg_non_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::coll_max_non_collection
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_max::top_level_all_coll_max
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_list_of_missing_element
  • partiql_tests::eval::primitives::coll_aggregate_function::coll_aggregate_function::coll_min::coll_min_top_level_agg_coll_min_distinct_data_result_success_1

partiql-parser/Cargo.toml Outdated Show resolved Hide resolved
#[track_caller]
fn coll_avg(elems: Vec<&Value>) -> Value {
if elems.is_empty() {
Null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these implementations return Null if the collection is empty. But I don't think any tests or conformance tests exist to verify this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They all return Null on an empty collection other than COLL_COUNT which returns 0. Those conformance tests were added in partiql/partiql-tests@f00114d, which doesn't seem to be in this PR. I'll update the test submodule to the more recent version.

@alancai98
Copy link
Member Author

Note: from #372, COLL_MAX and COLL_MIN should not permit comparisons over non-comparable types. This issue will be addressed in a subsequent PR.

@alancai98 alancai98 merged commit 637fdab into main May 25, 2023
@alancai98 alancai98 deleted the add-coll-fns branch May 25, 2023 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants