You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "timsaucer (via GitHub)" <gi...@apache.org> on 2024/05/05 14:44:27 UTC

[I] Incorrect conversion of pyarrow interval value to datafusion literal [datafusion-python]

timsaucer opened a new issue, #665:
URL: https://github.com/apache/datafusion-python/issues/665

   **Describe the bug**
   When creating a literal interval value from a pyarrow scalar, the values for month, day, and nanoseconds are not correctly assigned in the literal values. The following minimal example will reproduce. This _appears_ to be limited to `datafusion-python` and not the rust implementation.
   
   **To Reproduce**
   ```
   print("Setting 1 month interval:")
   pa_interval = pa.scalar((1, 0, 0), type=pa.month_day_nano_interval())
   print("pa_interval:", pa_interval)
   
   lit_interval = lit(pa_interval)
   print("lit_interval:", lit_interval)
   
   df.select(lit_interval).limit(1).show()
   
   print("Setting 1 day interval:")
   pa_interval = pa.scalar((0, 1, 0), type=pa.month_day_nano_interval())
   print("pa_interval:", pa_interval)
   
   lit_interval = lit(pa_interval)
   print("lit_interval:", lit_interval)
   
   df.select(lit_interval).limit(1).show()
   
   print("Setting 1 nanosecond interval:")
   pa_interval = pa.scalar((0, 0, 1), type=pa.month_day_nano_interval())
   print("pa_interval:", pa_interval)
   
   lit_interval = lit(pa_interval)
   print("lit_interval:", lit_interval)
   
   df.select(lit_interval).limit(1).show()
   ```
   
   Produces the following result:
   
   ```
   Setting 1 month interval:
   pa_interval: MonthDayNano(months=1, days=0, nanoseconds=0)
   lit_interval: Expr(IntervalMonthDayNano("1"))
   DataFrame()
   +-------------------------------------------------------+
   | IntervalMonthDayNano("1")                             |
   +-------------------------------------------------------+
   | 0 years 0 mons 0 days 0 hours 0 mins 0.000000001 secs |
   +-------------------------------------------------------+
   Setting 1 day interval:
   pa_interval: MonthDayNano(months=0, days=1, nanoseconds=0)
   lit_interval: Expr(IntervalMonthDayNano("4294967296"))
   DataFrame()
   +-------------------------------------------------------+
   | IntervalMonthDayNano("4294967296")                    |
   +-------------------------------------------------------+
   | 0 years 0 mons 0 days 0 hours 0 mins 4.294967296 secs |
   +-------------------------------------------------------+
   Setting 1 nanosecond interval:
   pa_interval: MonthDayNano(months=0, days=0, nanoseconds=1)
   lit_interval: Expr(IntervalMonthDayNano("18446744073709551616"))
   DataFrame()
   +-------------------------------------------------------+
   | IntervalMonthDayNano("18446744073709551616")          |
   +-------------------------------------------------------+
   | 0 years 0 mons 1 days 0 hours 0 mins 0.000000000 secs |
   +-------------------------------------------------------+
   ```
   
   **Expected behavior**
   When setting an interval value of 1 month in pyarrow, it should show up as 1 month in the datafusion data frame, and so on for the other values.
   
   **Additional context**
   Add any other context about the problem here.
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Incorrect conversion of pyarrow interval value to datafusion literal [datafusion-python]

Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed issue #665: Incorrect conversion of pyarrow interval value to datafusion literal
URL: https://github.com/apache/datafusion-python/issues/665


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Incorrect conversion of pyarrow interval value to datafusion literal [datafusion-python]

Posted by "Michael-J-Ward (via GitHub)" <gi...@apache.org>.
Michael-J-Ward commented on issue #665:
URL: https://github.com/apache/datafusion-python/issues/665#issuecomment-2099448911

   It's probably related to this issue in arrow-rs: [Rust Interval definition is incorrect](https://github.com/apache/arrow-rs/issues/5654).
   
   Here's a [godbolt link](https://godbolt.org/z/jaMn36Ghc) demonstrating the "1 month becomes 1 nanosecond" example. (I based that on [a comment in a similar thread in duckdb-wasm](https://github.com/duckdb/duckdb-wasm/issues/1696#issuecomment-2047272977)).
   
   I would suspect that if all code paths use the same impl, then `datafusion-python` wouldn't notice it, but perhaps that's wrong, or maybe not all code-paths use arrow-rs?
   
   The error occurs in the pyo3 magic as we cross the `python -> rust` bridge. Notice the python side assert's `pyarrow.Scalar`, but then the rust-side receives a `datafusion::ScalarValue`. (aside: is this magic type conversion intentional?)
   
   python side:
   https://github.com/apache/datafusion-python/blob/67d4cfb847e42724319aeec9014889262e6ea58a/datafusion/__init__.py#L182-L185
   
   rust side: 
   https://github.com/apache/datafusion-python/blob/67d4cfb847e42724319aeec9014889262e6ea58a/src/expr.rs#L229-L232
   
   The error is already present before the rust-method is invoked, adding print statements on both sides of the bridge:
   
   ```console
   converting: MonthDayNano(months=1, days=0, nanoseconds=0)
   converting: IntervalMonthDayNano("1")
   ```


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Incorrect conversion of pyarrow interval value to datafusion literal [datafusion-python]

Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed issue #665: Incorrect conversion of pyarrow interval value to datafusion literal
URL: https://github.com/apache/datafusion-python/issues/665


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] Incorrect conversion of pyarrow interval value to datafusion literal [datafusion-python]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on issue #665:
URL: https://github.com/apache/datafusion-python/issues/665#issuecomment-2107580388

   TODO: If the PR https://github.com/apache/datafusion-python/pull/666 merges in before this issues is corrected, the following examples in the `examples/tpch` folder will need to be updated
   
   
   - q01_pricing_summary_report.py
   - q04_order_priority_checking.py
   - q05_local_supplier_volume.py
   - q06_forecasting_revenue_change.py
   - q10_returned_item_reporting.py
   - q12_ship_mode_order_priority.py
   - q14_promotion_effect.py
   - q15_top_supplier.py
   - q20_potential_part_promotion.py


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org