Add support for LISTOF function, array return, and EXPLODE operator in PyDough - #523
Add support for LISTOF function, array return, and EXPLODE operator in PyDough#523knassre-bodo wants to merge 48 commits into
Conversation
| def user_collection(self) -> PyDoughUserGeneratedCollection: | ||
| """The wrapped user-generated collection.""" | ||
| return self._parcel[0] |
There was a problem hiding this comment.
This is not a thing that should be allowed with unqualified nodes, since any method or property name you add becomes something that can no longer be used as a PyDough term name.
…exploded data columns [RUN DIALECTS][RUN CI]
john-sanchez31
left a comment
There was a problem hiding this comment.
Great job Kian, almost there. Left some comments below :)
| - `name` (required): the name of the collection created from the explosion operation (similar to `PARTITION`). | ||
| - `value_name` (required): a string literal declaring the name of the new column that will be used to store the exploded data. | ||
| - `index_name` (optional): a string literal declaring the name of the new column that will be used to store the indices of the exploded data. If not provided, this column is not generated. The `index_name` is required if `is_distinct` is False. The indices are 0-indexed. | ||
| - `version` (optional, default=`"array"`): either `"array"` or `"string"`, stating whether the data to explode is an array being flattened or a string being split on a delimiter. |
There was a problem hiding this comment.
Is this argument really needed? Can't we just check the type of data?
There was a problem hiding this comment.
No because our typing system is not fully robust/reliable
| - `index_name` (optional): a string literal declaring the name of the new column that will be used to store the indices of the exploded data. If not provided, this column is not generated. The `index_name` is required if `is_distinct` is False. The indices are 0-indexed. | ||
| - `version` (optional, default=`"array"`): either `"array"` or `"string"`, stating whether the data to explode is an array being flattened or a string being split on a delimiter. | ||
| - `delimiter` (optional): a string literal indicating the delimiter that should be used to split up the string if `version="string"`. If `delimiter` is an empty string, the string will be split into individual characters. | ||
| - `filtering` (optional, default=`True`): `True` if it is possible for not every row in the original collection to be preserved in the exploded sub-collection (i.e. if one of the arrays is empty), and `False` otherwise. |
There was a problem hiding this comment.
I find this one kind of confusing. Will this filter on certain conditions or will be more like filtering Empty/None values? Assuming the second one, the description could be something as follow:
| - `filtering` (optional, default=`True`): `True` if it is possible for not every row in the original collection to be preserved in the exploded sub-collection (i.e. if one of the arrays is empty), and `False` otherwise. | |
| `filtering_empty` (optional, default=`True`): `True` will filter all empty or None values from the original collection to the exploded sub-collection, and `False` otherwise. |
There was a problem hiding this comment.
That's not what this does. It is just a boolean telling PyDough whether the operation will potentially remove rows (e.g., CAN any of the rows from the original be an empty array.
| infinity value with `DatabaseDiatect.MYSQL` an error will be raised. | ||
|
|
||
| > [!IMPORTANT] | ||
| > `ArrayType` is only supported for certain dialects: Trino, Postgres, DuckDB, Databricks. |
There was a problem hiding this comment.
What about Snowflake? I saw it among the supported dialects for the EXPLODE operator
There was a problem hiding this comment.
Array literals aren't a thing in Snowflake; you cannot put arrays inside VALUES
|
|
||
| ```py | ||
| # For each region, list the names of all nations inside that region | ||
| Regions.CALCULATE(region_name=name, nation_names=LISTOF(nations.name)) |
There was a problem hiding this comment.
Can we add how the result for each of the good example would look like?
There was a problem hiding this comment.
For the regions, sure. The other one is too big.
| idx_index: int | None, | ||
| lateral_alias: str, | ||
| subquery_alias: str, | ||
| ) -> SQLGlotExpression: |
There was a problem hiding this comment.
Can we add a docstring describing this implementation? What would be the final SQL for every dialect ?
| inner_term = "CAST('1970-01-01' AS TIMESTAMP)" | ||
| case _: | ||
| raise ValueError( | ||
| f"Cannot support empty array of type {inner_type} in Postgres." |
There was a problem hiding this comment.
Also can we add in the documentation what exactly is supported for Postgres as well?
There was a problem hiding this comment.
Will do. To be clear, this is a weird edge case where Postgres struggles to handle empty array literals, and this is a workaround that only works for certain types.
| if self.skip_sql: | ||
| pytest.skip(f"Skipping SQL text test for {self.test_name}") | ||
|
|
||
| if ( |
There was a problem hiding this comment.
I like this new functionality. Can you check if the skip of MySQL for infinity values, BodoSQL with to_table and DuckDb/Databricks with keywords can be skipped with this new property?
Linked ticket
Closes #521
Closes #522
Type of change
What changed and why?
Adds the
LISTOFfunction to PyDough to aggregate data into arrays, and theEXPLODEoperator to flatten such arrays into multiple rows of table data. Also added support for DataFrame collections containing arrays, which become array literals in the SQL.How I tested this?
array_data_to test the LISTOF and array literal features.explode_to test different variations of the EXPLODE operator, both on arrays and strings. Variations include:collection.EXPLODE(data)collection.CALCULATE(x=COUNT(EXPLODE(data)))collection.EXPLODE(...).EXPLODE(...))Notes for reviewers