[SPARK-53659][SQL] Infer Variant shredding schema when writing to Parquet #52406
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cashmand
changed the title
gene-db
suggested changes
Sep 29, 2025
| /** | ||
| * | ||
| * Infer a schema when there are Variant values in the shredding schema. | ||
| * Only VariantType values at the top level or nested in struct fields are replaced. |
There was a problem hiding this comment.
What are they replaced with?
| * Infer a schema when there are Variant values in the shredding schema. | ||
| * Only VariantType values at the top level or nested in struct fields are replaced. | ||
| * VariantType nested in arrays or maps are not modified. | ||
| * @param schema The original schema containing VariantType. |
There was a problem hiding this comment.
Does this require that the schema must contain a VariantType, and does the VariantType have to be a top-level field?
| .flatten | ||
| } | ||
|
|
||
| private def getValueAtPath(s: StructType, row: InternalRow, p: Seq[Int]): Option[VariantVal] = { |
There was a problem hiding this comment.
NIT:
Suggested change
| private def getValueAtPath(s: StructType, row: InternalRow, p: Seq[Int]): Option[VariantVal] = { | |
| private def getValueAtPath(schema: StructType, row: InternalRow, path: Seq[Int]): Option[VariantVal] = { |
Also, can we comment on what this will return? It looks it will return None of the field will be null?
| } | ||
|
|
||
| /** | ||
| * Update each VariantType with its inferred schema. |
There was a problem hiding this comment.
Does it return a copy of the schema (with the updates), or does it update in place?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
When writing Variant to Parquet, we want the shredding schema to adapt to the data being written on a per-file basis. This PR adds a new output writer that buffers the first few rows before starting the write, then uses the content of those rows to determine a shredding schema, and only then creates the Parquet writer with that schema.
The heuristics for determining the shredding schema are currently fairly simple: if a field appears consistently with a consistent type, we create
valueandtyped_value, and if it appears with an inconsistent type, we only createvalue. We drop fields that occur in less than 10% of sampled rows, and have an upper bound of 300 total fields (countingvalueandtyped_valueseparately) to avoid creating excessively wide Parquet schemas, which can cause performance issues.Why are the changes needed?
Allows Spark to make use of the Variant shredding spec without requiring the user to manually set a shredding schema.
Does this PR introduce any user-facing change?
Only if
spark.sql.variant.inferShreddingSchemaandspark.sql.variant.writeShredding.enabledare both set to true. They currently false by default.How was this patch tested?
Unit tests in PR.
Was this patch authored or co-authored using generative AI tooling?
No.