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

feat(api): add support for passing an optional index parameter to array map and filter #10205

Merged
merged 5 commits into from
Sep 25, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Sep 24, 2024

Description of changes

This PR adds support for passing an additional zero-based index argument to
the callable of array map and filter.

Some of this is a hellscape.

  • Postgres, one-based, so we apply a new rewrite rule to all uses of the index argument in the function body
  • DuckDB, one-based, so it gets the rule application
  • BigQuery: zero-based so pretty reasonable modification here
  • Snowflake: hellscape, because there's no native way to do this, so we have a snowflake-specific rewrite rule that transforms zipped arrays into struct field access.
  • ClickHouse: probably the most reasonable implementation, generate an index array and call arrayMap((x, i) -> do_stuff, arg, index).
  • Trino: not too bad, just some shenanigans in filter to filter values after calling zip_with(arg, range, return_null_if_predicate_is_false).

Issues closed

Closes #10201.

@cpcloud cpcloud added this to the 10.0 milestone Sep 24, 2024
@cpcloud cpcloud added feature Features or general enhancements postgres The PostgreSQL backend clickhouse The ClickHouse backend bigquery The BigQuery backend duckdb The DuckDB backend snowflake The Snowflake backend trino The Trino backend labels Sep 24, 2024
@github-actions github-actions bot added the tests Issues or PRs related to tests label Sep 24, 2024
@cpcloud cpcloud added the ci-run-cloud Add this label to trigger a run of BigQuery, Snowflake, and Databricks backends in CI label Sep 24, 2024
@ibis-docs-bot ibis-docs-bot bot removed the ci-run-cloud Add this label to trigger a run of BigQuery, Snowflake, and Databricks backends in CI label Sep 24, 2024
@cpcloud cpcloud added the ci-run-cloud Add this label to trigger a run of BigQuery, Snowflake, and Databricks backends in CI label Sep 24, 2024
@ibis-docs-bot ibis-docs-bot bot removed the ci-run-cloud Add this label to trigger a run of BigQuery, Snowflake, and Databricks backends in CI label Sep 24, 2024
Copy link
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Just a couple small nits, overall this LGTM!

ibis/expr/types/arrays.py Outdated Show resolved Hide resolved
ibis/expr/types/arrays.py Show resolved Hide resolved
ibis/backends/sql/rewrites.py Outdated Show resolved Hide resolved
ibis/backends/tests/test_array.py Show resolved Hide resolved
@cpcloud cpcloud added the pyspark The Apache PySpark backend label Sep 25, 2024
@cpcloud cpcloud force-pushed the array-map-and-filter-with-index branch from 6a3dfbd to 68275f6 Compare September 25, 2024 09:12
@cpcloud
Copy link
Member Author

cpcloud commented Sep 25, 2024

Clouds are passing:

…/ibis on  array-map-and-filter-with-index is 📦 v9.5.0 via 🐍 v3.12.5 via ❄️   impure (shell)
❯ pytest -m snowflake -n 8 --dist loadgroup --snapshot-update -qx && pytest -m bigquery -n auto --dist loadgroup --snapshot-update -q
bringing up nodes...
...............x............x........................x..........................................................................................x...........x...........x.x................................... [ 10%]
.............x.................................s....................................s......s............................................x....x......xx.......x.x..............x..x...x.......x.....x........xx [ 20%]
..xx....x......x...x..x....x.x.x.x...x......x.....x....x....x.........x.......x.x..x....x...x.........x.x......x...............s....................xx.............x................................x........x [ 31%]
...........x.....x...............................xx........x....s...........x.x...x..........x...xx...........................x..................x...x.................................................x...... [ 41%]
..................x....................................................................................................x................xx....x............xx........x.x.x.................................... [ 51%]
.......x.x.x........x..........x....xx...xxxx..............x..x........x....x...x....x..x...x......x.x....xx.........x..x...xx.....................x...x..............x...........................x....x..x... [ 62%]
.....................x.x.........x...........x..............x.................................xx..........................x...............................................x.x........x......x......xx....x..x. [ 72%]
...........x....x..x.....xx....x..............................................x.x................x............x.......x...sxxxx....xxx.x.x.xx.xx.x.xxxxxxxxxxx.x......x..xx.xxx..xx.x...x...x..xx.xx.xxxx..xxx [ 82%]
x.xxx..x..xxx..x.x.x..x.x..x.x.x...x....xx.x.xxx..x.xxxx...x......x..x..x.xxxxxx.x.s.x.xx.x..x................................................................................................................ [ 93%]
.......................................................s.....s..........................s................x.................................                                                                    [100%]
1755 passed, 10 skipped, 228 xfailed in 224.32s (0:03:44)
bringing up nodes...
..........................s..........................sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss..x...xx...x.s..ssX.....x..sssssssssssssssssssss...x.. [  9%]
.....x.........xx.......x.............x.........x...x..x..............x.......x.........x......x.x...x...x......x...........x.......................xx.....x.....xx....x.......................x...x...x...... [ 18%]
.....x...x........................................................x.........................x....................x............s.........x.........................x........................................x.. [ 27%]
.........................................x...................xx......x...x..x...x.......xxx.x...x.........x....x........xx.....xx..X..x...x...x.........xx......Xxxx......xXx....x.x...X...........xx.x..x...x [ 37%]
s...x.x...x............................s............................................x..s.....x.......x..........x.xxxx.x.x.....xx.x..xxxx...x.....x..xxx...x.xxx.x..x...x.x................x..x....x........x. [ 46%]
........x.xxx........x............x........xx.....x....x............x.........x...x..........x.............................x...........x....x..............xx..x...x................xxxxxxxxxxx...xxxx.x....xx [ 55%]
xxxxxxxx....x.........x......x..x.........x...x...x.....x..........................x.x.x...........x..................x.....x....................x.............x............x.......x..x..x.x...........xx.... [ 64%]
x..x...x.........x.......x.............x....x....x.......x...x...xx..x.xx...x.x............x.......x.........xx..x..........x......xx...x..x.x..x...x...x.......x.....x...x.x.......x..xx........xxxx......... [ 74%]
.x..x..xxxxx..xxxxx...xx.xxxxxxxxxxxxxxxxxxxxx.xxxx.xxxxxx.xx.xxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxx........x.......xx....................x..........x............x........x................. [ 83%]
...............................x........x....x..x...x.......x.x.xx.xxxxx..xxx.x.....x..............x...........................................................................x.............................. [ 92%]
....................................................................s.s.....x.......s......x...................s..........x.......................................                                             [100%]
1730 passed, 132 skipped, 355 xfailed, 5 xpassed in 324.38s (0:05:24)

@cpcloud cpcloud merged commit dfe7c34 into ibis-project:main Sep 25, 2024
77 checks passed
@cpcloud cpcloud deleted the array-map-and-filter-with-index branch September 25, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bigquery The BigQuery backend clickhouse The ClickHouse backend duckdb The DuckDB backend feature Features or general enhancements postgres The PostgreSQL backend pyspark The Apache PySpark backend snowflake The Snowflake backend tests Issues or PRs related to tests trino The Trino backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: allow the index as an argument to list functions such as filter and map
2 participants