fix: fall back when a struct repeats a Parquet field id - #6004
Conversation
|
this PR is supposed to be more generic, not only Parquet scan, but if UDF or any operator produces a schema with a struct having duplicated column we gonna fallback. Other PRs likely to be built on top of this? WDYT? @andygrove @dwsmith1983 @ErikBPF I expect the change should be straightforward as we know about the schema attached to the plan nodes on the planning side. |
sunchao
left a comment
There was a problem hiding this comment.
Correctness
This change extends duplicate-struct fallback to operator conversion and the Arrow cache serializer. Previously the shared type checker protected scans, shuffle, and row conversion, but an operator or cache could still expose a struct with repeated child names to Java Arrow. The new operator check covers both outputs and data-producing inputs, and the cache check consistently selects Spark's default cache format for unsupported structs.
Maintained Spark 3.5 and 4.0 resolve Parquet field IDs among siblings and raise when a requested ID matches multiple physical fields. That rule also applies at the file-schema root. The new scan guard catches nested duplicates but misses duplicate IDs on top-level fields, because the inherited schema entry point passes only each field's data type. This leaves a declared-schema case of the same equal-schema/no-predicate reader bypass unguarded. The first inline P2 describes the missing root check and regression case.
The second P2 is the missing CI registration for DataTypeSupportSuite. Both preflight runs fail on that omission, and the Spark runtime jobs are skipped. The new reader fixture usefully covers metadata-free files with field-ID matching enabled and disabled, while the cache test checks the selected batch format and readback. Those runtime results are author-reported, not independently established here.
Validation and scope
Reviewed head 204b6574 against base 74725611. The seven authored file changes are identical in merge 3dc05091, which also includes the newer base shuffle commit. Local python3 -B dev/ci/check-suites.py reproduces the preflight failure. An isolated compile of the exact shared type checker and extracted scan checker confirms that top-level duplicate IDs are accepted while nested duplicate IDs are rejected. That probe uses configuration/type-shim stubs and cached Spark type classes. Its later flag-disabled control stops on a missing Kryo class, so this is component evidence, not a passing Spark/JNI suite.
The maintained Spark 3.4 and 4.1 branches were unavailable for semantic comparison. Physical-only duplicate names hidden by a declared schema remain outside this PR's stated scope.
Performance
The added checks run during planning and cache-format selection rather than per row. Falling back prevents constructing Arrow batches whose struct children cannot be represented faithfully. The operator check short-circuits after the first duplicate and preserves ordinary repeated top-level output names.
The scan check recursively walks a type and then the existing support checker recursively visits it again, so deeply nested schemas repeat some work. Moving the ID validation to a single schema-level pass would also address the correctness gap. I have no benchmark evidence of a material planning regression, and no execution-speed claim follows from this review.
Design
A common conversion gate is a sensible place to enforce the Arrow boundary invariant. Inspecting child outputs matters for operators that consume a duplicate-named struct without returning it, and unwrapping WriteFilesExec follows the existing input handling. Using the serializer's support predicate for both cache creation and reading keeps the chosen cache format consistent.
Name duplication and Parquet ID ambiguity need different entry points. Repeated top-level output names can be valid positional columns, but duplicate field IDs at the file root still require the Parquet read check. Keep the current nested-name behavior and apply ID validation to the complete requested scan schema when field-ID matching is enabled.
Abstraction & complexity
The small recursive helper keeps struct, array, and map traversal together and produces a useful path in fallback explanations. It does not require a new framework or configuration surface. Malformed field-ID metadata is excluded from duplicate detection, while the existing Spark-backed schema serialization still validates the ID value.
The main simplification is to validate field IDs once at the schema boundary instead of recursively re-entering the same check through each type. The two inline P2s are the actionable changes from this review.
| // raising, so hand the read back to Spark and let it report the ambiguity. See #5801. | ||
| lazy val duplicateFieldIds = | ||
| if (CometParquetUtils.readFieldId(SQLConf.get)) { | ||
| DataTypeSupport.findDuplicateStructFieldIds(dt, name) |
There was a problem hiding this comment.
Correctness
[P2] Check duplicate field IDs on the complete requested schema
DataTypeSupport.isSchemaSupported calls this method with each field's dataType, so StructType(x: Long id=1, y: Long id=1) reaches this helper twice as LongType and is accepted. I confirmed that with an isolated compile of the exact checker. Wrapping the same fields inside s correctly falls back.
For a metadata-free Parquet file with those two top-level fields and the same requested schema, this leaves the same no-predicate bypass as the new nested fixture: DataFusion 55.1 skips the expression adapter when the schemas are equal. Spark's field-ID ambiguity check applies at the root as well. Please run this check once on the complete requested schema and add the flattened version of the new reader fixture. The exception for repeated top-level output names does not apply to Parquet field IDs.
There was a problem hiding this comment.
Fixed in 7fe9328, and thank you both — this was a real gap, not a nit. @andygrove's run at the root is exactly right, and the cause is structural as he says: isSchemaSupported hands isTypeSupported each field's data type, so the schema's own field list is never available to any override.
I took a slightly different shape than "run the check once over the whole requested schema", so flagging it for you to sanity-check. Rather than one recursive pass, CometScanTypeChecker now overrides both entry points and each does one shallow check of exactly one field list:
isSchemaSupported→ the schema's own fields (the root case that was missing)isTypeSupported→ each nested struct, as the trait's existing recursion reaches it
Same coverage, and every field list is visited exactly once, so it also removes the repeated subtree traversal @sunchao flagged under Performance.
Tests, both as requested:
CometScanRuleSuitenow leads with the unwrapped root schema, alongside a wrapped struct and a map value, plus distinct-id / one-id-absent / malformed-id acceptance controls and the flag-disabled control for each.CometNativeReaderSuitehas the flattened fixture as its own test:message spark_schema { optional int64 x = 1; optional int64 y = 1; }, no key-value metadata, read back with the matching schema. Comet declines and Spark reportsFound duplicate field(s) "1"; with field id matching off the scan stays native.
One more fix while in here: the root reason was rendering doubled, because the caller already wraps reasons in s"Unsupported schema ${requiredSchema}: ...". Dropped the redundant prefix.
I did consider the deeper fix of adding a field-list hook to DataTypeSupport itself so every checker gets one, and decided against it here. It needs an isRoot flag whose only job is to suppress the duplicate-name check at the root — top-level output attributes legitimately repeat a name, a self-join gives two id columns and Comet matches those positionally — and that risks silently changing behaviour for the shuffle, row-conversion and cache checkers in order to benefit one subclass. Happy to do it if you'd rather.
| * Arrow tell these struct children apart, and can Spark's Parquet reader resolve them one-to-one | ||
| * -- so the answers live in one place and are pinned here. | ||
| */ | ||
| class DataTypeSupportSuite extends AnyFunSuite { |
There was a problem hiding this comment.
Correctness
[P2] Register the new suite in both CI workflow matrices
dev/ci/check-suites.py requires every non-excluded suite to appear in both .github/workflows/pr_build_linux.yml and .github/workflows/pr_build_macos.yml. Neither contains org.apache.comet.DataTypeSupportSuite. The current preflight exits 255 with Suite not found in workflow .github/workflows/pr_build_linux.yml: org.apache.comet.DataTypeSupportSuite, which I also reproduced locally. This stops CI before the runtime test jobs run. Please add the suite to both matrices so the guard succeeds and the new unit tests execute.
There was a problem hiding this comment.
Resolved in 7fe9328, but by removing the suite rather than registering it — flagging that explicitly since you'd already verified the registration as fixed and I don't want this to read as a regression.
A cleanup pass found DataTypeSupportSuite largely duplicated CometNativeShuffleSuite:712, which already asserts duplicate names through isSchemaSupported, the array/map recursion, and the a-vs-A negative. Only two assertions were genuinely new — the malformed field id and the mixed present/absent id — and both moved into the CometScanRuleSuite table.
So the coverage still runs in CI: org.apache.comet.rules.CometScanRuleSuite is registered in both matrices already, and it exercises the predicates through the real entry point rather than directly. check-suites.py passes, one file and two registrations fewer.
Your underlying point stands and was the right catch — a new suite that CI never runs is worse than no suite.
204b657 to
a406fc7
Compare
andygrove
left a comment
There was a problem hiding this comment.
The force-push changed what this PR is, so flagging that up front. The operator conversion gate is gone, which means the "not only Parquet scan, any operator that produces a duplicate-named struct falls back" part that @comphead described above as the difference from #5786 is no longer in here. What is left is the scan check for duplicate field ids plus the cache serializer check. That is a reasonable PR on its own, but it means the "any operator" half of #5605 is now covered by nothing that is open. Is that coming back here, or should we file it separately so it does not get lost?
One note if it does come back. I ran the earlier head 204b6574 and that gate broke CometArrayExpressionSuite "folded map value with duplicate struct field names falls back (multirow)". The plan still falls back and the answer is still right, but the gate fires before the serde runs, so the recorded reason becomes Native operators do not support v.map value: struct with duplicate field names (x) and the expected Unsupported data type MapType is never recorded. That suite was not in the list you ran locally.
Preflight is red on both runs with Suite not found in workflow .github/workflows/pr_build_linux.yml: org.apache.comet.DataTypeSupportSuite. dev/ci/check-suites.py scans pr_build_macos.yml too, so it needs adding to both. Everything downstream is skipped right now, so there is no runtime coverage of this change in CI yet.
One more small thing that did not fit on a line in the diff. The description says the point of hasDuplicateFieldNames is that the definition of "duplicate name" lives in one place, but CometShuffleExchangeExec.scala:483 and CometShuffleExchangeExec.scala:610 still spell it out by hand as fields.map(f => f.name).distinct.length == fields.length. Could those use the new helper while you are in here?
I ran this locally on the Spark 4.1 profile, JDK 17, macOS aarch64, at a406fc75. Both of your new runtime tests pass, and so does CometExpressionSuite "named_struct with duplicate field names". The root-level field id case in my inline comment below fails.
| // carrying its id, and raises when more than one answers. A requested struct that repeats an | ||
| // id cannot be resolved that way, and the native scan reads it positionally rather than | ||
| // raising, so hand the read back to Spark and let it report the ambiguity. See #5801. | ||
| lazy val duplicateFieldIds = |
There was a problem hiding this comment.
I confirmed @sunchao's point about top-level fields with a real run rather than a compile probe, and it does reproduce on this head.
Same construction as your new test in CometNativeReaderSuite, but with the two id-1 fields at the top level instead of inside s:
message spark_schema {
optional int64 x = 1;
optional int64 y = 1;
}
read back with StructType(Seq(withFieldId("x", 1), withFieldId("y", 1))) and spark.sql.parquet.fieldId.read.enabled=true:
COMET: ROWS=[10,20] (CometNativeScan in the plan)
SPARK: ERROR=Found duplicate field(s) "1": [x, y] in id mapping mode.
That is #5801 exactly, one level up. The reason is structural rather than an oversight in the predicate: DataTypeSupport.isSchemaSupported hands isTypeSupported each field's dataType, so the root struct's fields are never compared against each other and this override cannot see them.
Would you consider overriding isSchemaSupported here instead and running findDuplicateStructFieldIds once over the whole requested schema? That covers the root, and it also drops the repeated traversal, since findDuplicateStructFieldIds already recurses while isTypeSupported re-enters it at every nesting level. Could the root case get a test alongside the nested one?
There was a problem hiding this comment.
Fixed in 7fe9328, and thank you both — this was a real gap, not a nit. @andygrove's run at the root is exactly right, and the cause is structural as he says: isSchemaSupported hands isTypeSupported each field's data type, so the schema's own field list is never available to any override.
I took a slightly different shape than "run the check once over the whole requested schema", so flagging it for you to sanity-check. Rather than one recursive pass, CometScanTypeChecker now overrides both entry points and each does one shallow check of exactly one field list:
isSchemaSupported→ the schema's own fields (the root case that was missing)isTypeSupported→ each nested struct, as the trait's existing recursion reaches it
Same coverage, and every field list is visited exactly once, so it also removes the repeated subtree traversal @sunchao flagged under Performance.
Tests, both as requested:
CometScanRuleSuitenow leads with the unwrapped root schema, alongside a wrapped struct and a map value, plus distinct-id / one-id-absent / malformed-id acceptance controls and the flag-disabled control for each.CometNativeReaderSuitehas the flattened fixture as its own test:message spark_schema { optional int64 x = 1; optional int64 y = 1; }, no key-value metadata, read back with the matching schema. Comet declines and Spark reportsFound duplicate field(s) "1"; with field id matching off the scan stays native.
One more fix while in here: the root reason was rendering doubled, because the caller already wraps reasons in s"Unsupported schema ${requiredSchema}: ...". Dropped the redundant prefix.
I did consider the deeper fix of adding a field-list hook to DataTypeSupport itself so every checker gets one, and decided against it here. It needs an isRoot flag whose only job is to suppress the duplicate-name check at the root — top-level output attributes legitimately repeat a name, a self-join gives two id columns and Comet matches those positionally — and that risks silently changing behaviour for the shuffle, row-conversion and cache checkers in order to benefit one subclass. Happy to do it if you'd rather.
| fields.map(_.name).distinct.length != fields.length | ||
|
|
||
| /** | ||
| * Describes the first struct nested anywhere in `dt` whose children repeat a Parquet field id, |
There was a problem hiding this comment.
Small thing about this doc comment. ParquetReadSupport.matchIdField raises when the id a requested field asks for matches more than one field in the file, whereas this checks whether two requested fields share an id. Those are different predicates, and they only coincide under the equal-schema condition #5801 describes.
That is enough for the gap you are closing, but the comment reads as though the second follows from the first, and someone will rely on that later. Could it say which one is implemented and why the narrower check is sufficient here?
There was a problem hiding this comment.
Good catch, and you're right that the comment implied the second followed from the first. Rewritten in 7fe9328 to lead with the distinction:
* True when two of `fields` declare the same Parquet field id.
*
* Deliberately not Spark's check: `ParquetReadSupport.matchIdField` raises when one *requested*
* id is carried by several fields *in the file*. The two coincide only when the requested
* schema equals the file schema, which is exactly when DataFusion's opener skips the expression
* adapter that would have validated the lookup (#5801). So this can decline a read Spark would
* have accepted, costing native execution but not correctness; it cannot report a duplicate
* Spark would not. File-side ambiguity is left to #5786.
*
* Only meaningful under `spark.sql.parquet.fieldId.read.enabled`; callers gate on that.
It now says which predicate is implemented, why the narrower one is sufficient here, and which direction the error can go — it can cost native execution but not correctness, and it cannot invent a duplicate Spark would not report.
| val MAP_VALUE = "map value" | ||
|
|
||
| /** Spark's `StructField` metadata key for a Parquet field id. */ | ||
| private val FIELD_ID_METADATA_KEY = "parquet.field.id" |
There was a problem hiding this comment.
Comet already uses ParquetUtils.hasFieldId and ParquetUtils.getFieldId for this in serde/operator/package.scala:48-51 and QueryPlanSerde.scala:700. Any reason not to use ParquetUtils.FIELD_ID_METADATA_KEY and ParquetUtils.hasFieldId here rather than redeclaring the key and hand-rolling the metadata read? It would keep the two from drifting if Spark ever changes it.
There was a problem hiding this comment.
No good reason — done in 7fe9328. fieldId now uses ParquetUtils.hasFieldId and ParquetUtils.getFieldId, and the redeclared FIELD_ID_METADATA_KEY is gone.
The only wrinkle is that getFieldId raises on a non-integral or out-of-range id, and this runs during planning where raising would be the wrong error, so it is wrapped:
private def fieldId(field: StructField): Option[Int] = {
if (!ParquetUtils.hasFieldId(field)) None
else {
// A malformed id is not this check's business to report -- getFieldId raises on one -- so
// treat it as absent and let the reader complain about it.
try Some(ParquetUtils.getFieldId(field))
catch { case _: IllegalArgumentException => None }
}
}Still Spark's own definition of what a field id is, which is the drift you were guarding against. There's a malformed id case in the CometScanRuleSuite acceptance controls pinning that it is not misreported as a duplicate.
Related, from the same cleanup: hasDuplicateFieldNames was extracted to be shared, but two inline copies of fields.map(f => f.name).distinct.length == fields.length were still sitting in CometShuffleExchangeExec at lines 483 and 610. Those now call the shared predicate. I left CometBatchKernelCodegen:103 alone — it works off st.fieldNames with a deliberate comment about not rebuilding that array, and converting it would reintroduce the allocation the comment avoids.
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed a406fc75 against 3b942e59. The generic operator conversion gate is removed, while the Parquet field-ID guard and Arrow cache fallback remain. The ID helper is simpler and its unit cases now include map keys and deeper nesting. I found no new P1/P2 issues in this narrower scope.
Both published P2s remain:
- Root field IDs:
isSchemaSupportedstill passes each field's type separately, so the root IDs are never compared. The exact-checker probe accepts root duplicates with field-ID reading enabled, rejects nested duplicates, and accepts distinct IDs and flag-disabled controls. The native adapter remains bypassed for equal schemas without a predicate. - Suite registration:
DataTypeSupportSuitestill exists and is absent from both workflow matrices. Localcheck-suites.pyexits 255, matching current preflight.
The probe passed eight schema-entry controls and five helper checks using cached Spark type classes and configuration/type-shim stubs. This is component evidence, not a Spark/JNI run. Maintained Spark 3.5/4.0 still require the ambiguity error at the root. Maintained 3.4/4.1 sources were unavailable. CI checked merge 09ec1e57, which preserves all six authored files, but preflight stopped it before runtime tests.
|
#5786 we would still need it, the rabbit hole is deeper than I expected. Claude led me to Spark writer issues which can be investigated and fixed later. |
a406fc7 to
3df2a8b
Compare
|
Created DF issue apache/datafusion#25433 |
3df2a8b to
806d909
Compare
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed 806d9098 against d0c10f2d. The ID helper is now a shallow predicate using the existing type recursion, which removes repeated subtree traversal. The new scan-rule tests cover wrapped structs, arrays and map values. I found no new P1/P2 issues.
The suite-registration P2 is fixed. DataTypeSupportSuite is listed in both workflow matrices, local check-suites.py passes, and current preflight passes on merge f680e09b.
The P2 for root field IDs remains. isSchemaSupported still passes each field's type separately, so root IDs are never compared. Every new test shape is wrapped inside col. The fresh exact-checker probe accepts root duplicates with field-ID reading enabled and rejects nested duplicates. The schema-level check and flattened reader fixture are still needed.
The probe passed 16 schema-entry controls and five helper checks, including map keys, deeper nesting, distinct IDs and flag-disabled cases. This is component evidence with configuration/type-shim stubs and cached Spark type classes, not a full Spark/JNI run. Maintained Spark 3.5/4.0 still require the root ambiguity error. Maintained 3.4/4.1 sources were unavailable. Runtime CI is still in progress. The tested merge retains the seven source/test files exactly and both workflow registrations alongside newer base suites.
Under `spark.sql.parquet.fieldId.read.enabled` Spark resolves each requested field to the one Parquet field carrying its id, and raises FOUND_DUPLICATE_FIELD_IN_FIELD_ID_LOOKUP_MODE when more than one answers. Comet never looked at field ids, so a requested schema that repeats one was read positionally and returned rows where Spark raises. The ids ride in the requested schema's `StructField.metadata`, so this is decidable from the plan. `DataTypeSupport` gains a `hasDuplicateFieldIds` predicate beside the existing duplicate-name one, and `CometScanTypeChecker` declines a field list that trips it. Both entry points are overridden, because they see different things. `isTypeSupported` is handed each field's data type, so it sees nested structs as the trait's recursion reaches them but can never compare two top-level fields; `isSchemaSupported` is where the schema's own field list is available. Each list is inspected exactly once, so nothing is re-walked at an enclosing level. `ArrowCachedBatchSerializer.supportsType` also accepted a struct with duplicate child names, so caching such a relation stored it in Comet's Arrow format, which Java Arrow cannot import back because it keys struct children by name. One more schema check delegates it to Spark's default cache format, alongside the interval types already excluded there, and the two copies of that predicate in the shuffle gate now call the shared one rather than spelling it out inline. No Parquet decoding changes; see apache#5786 for that path. Closes apache#5801.
806d909 to
7fe9328
Compare
|
@andygrove on the operator gate — it is intentionally out of this PR, and it should be filed separately so it does not get lost. I'd rather not bring it back here: this PR is now just "check the schema, fall back", and the operator gate is a different mechanism with a different blast radius. Your Worth recording what I found while checking whether this PR could also cover #5783, because it changes what a fix there should aim for. I compared four readers on the file Spark's writer produces for
Two things follow. Comet reads a duplicate-named struct correctly when the file is well formed. On the DataFusion-written file — same duplicate The file Spark writes is internally inconsistent: the row group declares 3 rows while each of the two same-path Practical consequence for #5786: rejecting duplicate Parquet field names before decoding would make Comet refuse the DataFusion-written file, which it currently reads correctly and in agreement with Spark. If a guard is wanted there, keying it on the Separately, DataFusion answers |
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed 7fe93280 against 23a9ff64. The root-field-ID P2 is fixed: the schema entry point now checks root IDs, and the flattened metadata-free Parquet fixture covers the earlier failure. The suite-registration P2 remains resolved after removing the duplicate suite and moving its unique cases into the registered scan-rule suite.
The shallow root and nested checks avoid repeated subtree traversal. The shared name predicate preserves shuffle behavior, and the metadata helper now uses Spark's field-ID APIs. No remaining P1/P2 findings.
Validation: the fresh component probe passed 20 schema controls and seven helper checks, including the original root case, flag-off behavior and unambiguous projections. Local suite-registration checks pass. On merge 3a9184d8, the scan job passed both root and nested reader fixtures, and the execution job passed the scan-rule and cache regressions. All seven changed source/test files match the reviewed head.
Local validation was a component probe with explicit stubs, not a full Spark/JNI run. Maintained Spark 3.5/4.0 semantics were checked. Local maintained 3.4/4.1 sources were unavailable.
Which issue does this PR close?
Closes #5801.
Rationale for this change
Comet has to match Spark. Under
spark.sql.parquet.fieldId.read.enabledSpark resolves each requested field to the one Parquet field carrying its id, and raisesFOUND_DUPLICATE_FIELD_IN_FIELD_ID_LOOKUP_MODEwhen more than one answers. Comet never looked at field ids at all, so a requested struct that repeats an id was read positionally and returned rows where Spark raises.The ids are in the requested schema's
StructField.metadata, so this is decidable from the plan. The fix is a schema check and a fallback, nothing more.What changes are included in this PR?
DataTypeSupportgainshasDuplicateFieldIds, a predicate beside the existing duplicate-name one. The existing name check is refactored to call the sharedhasDuplicateFieldNamesso the two read the same way.CometScanTypeCheckerdeclines a struct that repeats a field id, when field id matching is on.DataTypeSupport's recursion carries the check into nested structs, arrays and maps, so the checker itself stays a shallow predicate.That is the whole change: check the schema, fall back. No Parquet decoding, no footer reads, no operator-level gates.
How are these changes tested?
DataTypeSupportSuite(new): both predicates, including that names differing only by case are not duplicates (Java Arrow tellsaandAapart), that a field with no id cannot collide, and that a non-integral id is left for Spark'sParquetUtils.getFieldIdto reject rather than misreported as a duplicate.CometScanRuleSuite: the checker declines a duplicate-id struct at every nesting level (struct child, array element, map value), and accepts all of them again withspark.sql.parquet.fieldId.read.enabled=false, where the fields are told apart by name.CometNativeReaderSuite: end to end. A Parquet file with no key-value metadata and schemas<x id=1, y id=1>, read back with that same schema — which is what makes DataFusion's opener skip the expression adapter. Comet now raises Spark'sFound duplicate field(s) "1": [x, y] in id mapping modeand the plan carries noCometNativeScanExec. With field id matching off the scan stays native and returns the row.Run locally on the Spark 4.1 profile:
DataTypeSupportSuite,CometScanRuleSuite,CometExecRuleSuite,CometNativeReaderSuite,CometNativeShuffleSuite,CometInMemoryCacheSuite,CometShuffleSuite,DisableAQECometShuffleSuite,ParquetReadV1Suite,CometFuzzTestSuite.cargo fmt --checkandcargo clippy --all-targets --workspace -- -D warningsclean (no Rust changes).Note on #5783, and a warning for #5786
While checking whether this PR could also cover #5783, I compared four readers on the same file. It turns out Comet has no duplicate-struct-field-name bug, and the guard #5786 proposes would cause a regression.
named_struct('dup', id, 'dup', id + 100)written by Spark produces a file that is internally inconsistent — the row group declares 3 rows while each of the two same-paths.dupleaf chunks declares 6 values:parquet-mr cannot read it at all (
ParquetEncodingException: [s, dup] -(0)-> 2). Spark and DuckDB clamp to 3 rows and return0, 100, 1, which is the two children interleaved, not the original data. Comet followsnum_valuesand returns 6. So there is no correct answer to match on that file; Spark's row count is right and its values are not.Writing the same logical data with a correct writer shows Comet is fine:
num_values/num_rows0,100,1count(*)says 3)dup_1)On the DataFusion-written file — same duplicate
dup/dupstruct, samespark.read.schema("s struct<dup: bigint>")read — Spark and Comet both return[0], [1], [2]withCometNativeScanExecin the plan. Exact match. DuckDB refuses to construct the shape at all (Binder Error: Duplicate struct entry name "dup").Two things follow:
num_valuesvsrow_group_num_rowsinconsistency, not on duplicate names.Separately, DataFusion answers
count(*)from row-group metadata and the scan fromnum_values, so on the malformed file it returns 3 and 6 in the same session, even with the scan wrapped in a subquery. That is worth an upstream issue and is independent of Comet.