You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/04/25 19:04:19 UTC
[PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
andygrove opened a new pull request, #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090
## Which issue does this PR close?
Closes #4089.
## Rationale for this change
When the `native_datafusion` scan reads a Parquet column with a higher-precision decimal physical type under a requested read schema with smaller precision or different scale, the existing schema adapter falls through to Spark's `Cast` expression. `Cast` happily rescales or truncates, producing wrong values silently. Spark's vectorized reader rejects this with `SchemaColumnConvertNotSupportedException`, and `native_iceberg_compat` already does the same via `TypeUtil.checkParquetType`. The native scan should match.
## What changes are included in this PR?
`native/core/src/parquet/schema_adapter.rs`: in `replace_with_spark_cast`, add a guard before the existing branches that returns a `DataFusionError::Plan` when both `physical_type` and `target_type` are `Decimal128` and either the target precision is smaller than the source precision or the scales differ. The rule mirrors Spark's `TypeUtil.isDecimalTypeMatched` (Spark 3.x path).
The check is intentionally narrow: it only fires for decimal-to-decimal mismatches, leaving every other type path unchanged. The closed prior attempt at broad schema validation in #3311 broke unrelated tests; this one does not.
Spark 4.0's decimal type widening (allowing strictly-larger target precision/scale per SPARK-37147) is not implemented here. For #4089's specific case (`Decimal(10,2)` read as `Decimal(5,0)`) Spark 4.0 also rejects, so behavior is correct on all supported versions. Implementing 4.0's wider widening is a follow-up.
## How are these changes tested?
Added a focused test to `ParquetReadSuite`: `native_datafusion rejects incompatible decimal precision/scale`. It writes `Decimal(10, 2)` data, reads it under `Decimal(5, 0)`, forces `spark.comet.scan.impl=native_datafusion` and `spark.sql.sources.useV1SourceList=parquet`, and asserts that `collect()` raises `SparkException`. Verified against `ParquetReadV1Suite` (44 tests, all pass; 1 pre-existing test ignored). The behavior is also covered by the per-impl matrix added in #4087, which will need its `decimal(10,2) read as decimal(5,0): native_datafusion` assertion flipped from "succeeds" to "throws" in a follow-up once that PR merges.
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4322214264
> In a case where we expect an exception to be generated anyway, can we catch this at CometScanRule rather than going all the way to serialization and native operators?
I think the issue is that we do not know the types of all the parquet files until runtime?
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4330989797
Merged. Thanks @mbutrovich
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4322252129
> I think the issue is that we do not know the types of all the parquet files until runtime?
IIRC from looking at this a while back, Spark has read the physical schema already, but thrown it away by the time our Comet rules run with no good way to get it again.
I'm not opposed to handling it this way, just wanted to think through if we could catch it earlier. It's also a fairly uncommon scenario.
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4322208711
In a case where we expect an exception to be generated anyway, can we catch this at CometScanRule rather than going all the way to serialization and native operators?
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4329598356
Expanded the comment in `replace_with_spark_cast` to document both the precision-only and scale-widening cases and to reference `isDecimalTypeMatched` directly. Pushed in fcbec7681.
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4327872544
Thanks @andygrove! I double-checked Spark's logic and the scale-narrowing guard looks right for preventing silent truncation.
Spark's `isDecimalTypeMatched` (line 1169 of `ParquetVectorUpdaterFactory.java`) requires exact scale match, so scale widening like Decimal(10,2) -> Decimal(10,4) would also be rejected by Spark's vectorized reader but passes through here. The PR description explains why that's fine (lossless cast), which makes sense. Would it be worth noting that in the code comment so future readers don't have to check the PR description?
--
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: [PR] fix: reject incompatible decimal precision/scale in native_datafusion scan [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #4090:
URL: https://github.com/apache/datafusion-comet/pull/4090#issuecomment-4322323338
> I'm not opposed to handling it this way, just wanted to think through if we could catch it earlier. It's also a fairly uncommon scenario.
I do think this is an edge case that is fairly unlikely IRL because it only happens when the user provides a schema that is incompatible with the file schema
--
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