Re: [PR] fix: prevent silent overflow when reading Parquet TIMESTAMP_MILLIS values [datafusion-comet]

["peterxcli (via GitHub)" <gi...@apache.org>]

peterxcli commented on PR #5177:
URL: https://github.com/apache/datafusion-comet/pull/5177#issuecomment-5159927478

   @andygrove thanks for the review! review change is pushed. please take another look, thanks!
   
   
   > 2. `div_euclid` is a behavior fix, not a refactor.
   
   Retitled the PR. The final patch rejects that invalid read-schema pair instead of using `div_euclid`.
   
   > 3. Refresh the PR description and mention overflow.
   
   Updated it to describe the planning rejection and that millis→micros overflow now errors instead of wrapping.
   
   > 4. Is micros→millis reachable from Spark or Iceberg?
   
   No. Spark read schemas use microsecond logical timestamps. Construction now returns `DataFusionError::Plan`, with a Spark source link.
   
   > 5. Keep scalar target-timezone coverage.
   
   The pair is now rejected before scalar or array evaluation. The planning test covers timezone-free and timezone-bearing fields.
   
   > 6. Use `DEFAULT_CAST_OPTIONS` at the third site.
   
   Done. The timezone-free `Date32 -> Timestamp` path now uses `DEFAULT_CAST_OPTIONS`.
   
   > 7. Explain why millis→micros always errors on overflow.
   
   Added links to Spark’s [Parquet call site](https://github.com/apache/spark/blob/v4.2.0/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java#L817-L833) and checked [`millisToMicros`](https://github.com/apache/spark/blob/v4.2.0/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/SparkDateTimeUtils.scala#L103-L108) implementation.


-- 
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