Repository navigation
Conversation
Test-only RED suite for metadata-preserving generic_project projection: natural stored-view regressions via an in-memory metastore fixture (RexInputRef, RexFieldAccess, RexCall and nested-view operands, both branch orders, multi-branch, nullability, defaults, record/field metadata, logical/fixed/enum, ambiguity and incompatibility cases), structural n-ary union and malformed-call tests, and D1 union alignment/compatibility checks in SchemaUtilitiesTests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…NITEM-25474) Typed generic_project calls over real converted scans: a non-string target map key must be rejected with field context, and a projected record keeps its DECIMAL leaf identity while a different requested scale fails explicitly. Natural SQL cannot reach the nested decimal case because the existing rewriter cannot express DECIMAL targets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…M-25474) - Project record defaults inside arrays and maps, including retained-field reordering through a typed call (F1). - Cover generated-projection operands through ITEM and through field access on a metadata-free UDF result, reject an ambiguous segment under ITEM, and pin ordinary non-helper ambiguous access as unchanged (F2). - Compare both shared Address definitions standalone, since Avro prints the second as a reference and record equality ignores docs (F3). - Reject an opaque-union reshape in the projection itself, both structurally and before the natural UNION merge (F4). - Carry enum aliases and typed properties through strict and non-strict normalization in both branch orders (F5). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pin array-of-record and map-of-record defaults through non-strict view normalization: plain base/evolved views keep every field and default, both UNION orders keep the canonical branch's exact defaults, and a reordered typed projection keeps its reshaped defaults after setupNameAndNamespace. Today these fail with the pre-existing "Unknown datum class: GenericData$Record" default-copy error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…TEM-25474) Per the planner's section 7 ruling: - Update 22 existing expected schemas (25 consuming tests) by default presence only: add 97 explicit null defaults already declared by the source Avro or created by unchanged Hive-fallback/partition/derived inference, and remove the two null defaults on Key/VALUE that base-casepreservation.avsc never declares. No other attribute changes. - T23: the outer u.b.pagekey is an ordinary field access, so its output name follows the existing policy (pagekey). Add a check that the fuzzy UNION below it still carries b.pageKey, and a plain-subquery control for the same ordinary access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ion (ACTIONITEM-25474) A fuzzy UNION branch wrapped in generic_project was rebuilt from the lower-cased Calcite type, so the projected branch no longer matched the case-preserved Avro of the other branch and the merge failed. - Infer the helper's operand (column, field access, ITEM/ordinal-return call, or inlined expression) in isolation, then project it onto the typed return with a new strict AvroSchemaProjection that keeps source spelling, docs, aliases, props, nullability and projected defaults. Missing, ambiguous (Foo/foo) or incompatible fields fail with their path. - UNION merge: exact-name bijections stay valid, unique case-only differences align to the first branch, ambiguity is rejected; fixed-size, logical-type and opaque-union conflicts now fail; enum/container/record metadata is kept; all inputs of an n-ary union are folded. - Copy and namespace normalization keep record/field/enum/container metadata and exact default presence (explicit null, no fabricated null, complex array/map-of-record defaults). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…TEM-25474) Final-review F1: a typed generic_project must not keep a source leaf schema when the requested relational type has different parameters. Over real converted scans, requesting BINARY(8) for a 16-byte Avro fixed (top-level and nested) or TIMESTAMP(6) for a timestamp-millis leaf must be rejected with the field path and requested type. Requesting every retained leaf with its own relational type remains a pure projection (control). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…amp/decimal parameters (ACTIONITEM-25474) sameLeafRepresentation treated any two types with the same SqlTypeName as the same representation. Compare precision and scale for the types where they define the Avro representation (BINARY fixed size, TIMESTAMP unit, TIME, DECIMAL); the coarse Hive string/binary cases are unchanged and nothing is cast. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… representation (ACTIONITEM-25474) A request that spells out BINARY/TIMESTAMP/TIME/DECIMAL parameters is compared with the fixed size, timestamp unit or decimal precision/scale of the Avro schema itself, not only with the source relational type. Parameterless requests keep the coarse Hive behavior. The rejection shows the Avro leaf schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F3 (planner section 8): a retained containing default constrains
nested nullable options. Natural fuzzy views in both orders, strict and
non-strict: canonical c {"x":1,"inner":{"y":2}} over required children
widened by the other branch must yield int-first children with no new field
defaults; the same through array elements and map values with several
entries; reversing keeps the canonical null-first options and defaults.
Controls: a child's own default is kept beside a different containing
member value, and a discarded branch's default imposes no order.
F1 follow-up: different fixed lengths and timestamp precisions are rejected
through optional, array and map positions and in the opposite direction;
BINARY(16) and TIMESTAMP(3), which match the Avro identity, are kept.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ITEM-25474) The merged type of a field can widen a required child to a nullable option. Retained containing defaults (record, array element and map value members, plus the declared defaults of nested fields) now constrain the option order recursively before the field is cloned: null needs null first, any other value needs its type first. Only the two alternatives of a nullable option are reordered; defaults are never coerced, dropped or invented, and conflicting retained demands are rejected with their path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
User-directed D6 (revision 3): non-strict stored-view namespace reconstruction keeps the legacy omission of RECORD and ENUM aliases. - T25: plain and one-branch fuzzy views (both orders) over sources with namespace-free and qualified aliases on top-level, nested, array, map and union records/enums. Non-strict outputs have empty resolved RECORD/ENUM alias sets before and after reparse and cannot read writers named by a former alias; FIXED (field-level and in an array) and field aliases stay intact. Strict views and RelNode conversion without view normalization keep every alias, with compatible writers before and after reparse (forceLowercase=false). - Correct non-strict oracles only: drop the record aliases in meta.avsc and the enum alias in logical.avsc, assert the empty sets in T11/T12, and clarify T4's field-alias assertion. Strict oracles are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
D6-T1: convert the fuzzy alias views' RelNodes directly, before any stored-view normalization. The non-strict UNION's pre-merge reconstruction must already omit RECORD/ENUM aliases (namespace-free and qualified, both orders) under the existing pre-merge identity policy, keeping FIXED and field aliases, before and after reparse. A plain RelNode conversion still keeps every alias; the class comment now distinguishes the two boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ization (ACTIONITEM-25474) Non-strict normalization has always rebuilt records and enums in their new namespace without their declared aliases (they are qualified by the namespace being replaced). The shared metadata copiers now preserve aliases, so restore that behavior only at the existing non-strict rebuild boundaries (setupNameAndNamespace / modifySchemaBeforeMerge recursion). Strict mode, the projector and shared copiers keep aliases; FIXED and field aliases, enum props/symbols/defaults and all other metadata are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F4: when a containing default forces a widened child to be
int-first, the rebuilt record must still accept its unchanged retained
siblings. Natural fuzzy view with c default {"x":1,"label":"kept"} and an
array default whose element records keep an unchanged tag: both strict modes
must keep the exact schema (x int-first, label/tag doc, alias and property
unchanged, no new field defaults) and both containing defaults. The reverse,
null-first canonical order stays a control.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F6: decimal(10) with an omitted scale and decimal(10,0) are the same Avro logical type (scale defaults to 0; both DECIMAL(10, 0)). A natural direct/direct stored-view UNION must accept them in both strict modes and both orders, at the top level and nested, keeping the canonical branch's own declaration exactly. Different effective scale or precision still fails with the field path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F5 (tester-clarifications section 11): VARBINARY(n) is a maximum. Typed generic_project calls over real converted scans: VARBINARY(8) over a 16-byte fixed must be rejected with the field path and both sizes, also through optional and array positions; VARBINARY(16), VARBINARY(32) and unbounded VARBINARY keep the fixed exactly. Over Avro bytes no bound is proven, so a finite VARBINARY(n) must be rejected (direct, optional, array). Controls: unbounded VARBINARY and unparameterized BINARY keep bytes, BINARY(8) over bytes stays rejected, and a VARCHAR request over a string keeps the existing behavior. Typed inference-boundary checks only; no natural VARBINARY flow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- F4: every field of a record rebuilt while ordering nullable options by retained defaults is an exact-presence copy, including unchanged siblings (a field can belong to one record only). - F5: a finite VARBINARY(n) request is a maximum length: over an Avro fixed(N) it is rejected when n < N and keeps the unchanged fixed otherwise; over bytes it is rejected as an unproven narrowing. The type-only equivalence helper does not treat a new finite bound as proven. Unbounded VARBINARY, BINARY, BINARY(n) and CHAR/VARCHAR behave as before. - F6: decimal logical types compare the effective scale (a missing scale is 0); the canonical left branch keeps its raw declaration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F7: a fixed12 carrying logicalType duration is coherently inferred as BINARY, and its fixed size proves the bound regardless of the annotation. Typed generic_project calls requesting VARBINARY(12) and VARBINARY(16) must keep the fixed exactly (annotation, property and field doc), top-level and inside an option. VARBINARY(8) stays rejected with the path and size as a separate guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ARY request (ACTIONITEM-25474) The finite VARBINARY(n) check required the Avro leaf to carry no logical type, which rejected a coherent binary operand backed by an annotated fixed (for example duration, fixed(12)). The fixed size N is the proven bound regardless of its annotation: reject n < N, keep the unchanged fixed (annotation, properties, identity) otherwise. Decimal is still not treated as a binary operand and bytes still proves no bound. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Final-review F8 (tester-clarifications section 12, conditions F8-1..F8-5): every use of one output named record must resolve to one complete definition whose nullable option order satisfies all retained containing defaults together. Natural fuzzy views over com.linkedin.f8.Shared, both strict modes. Every in-memory occurrence is compared with the single expected definition (doc, aliases, properties) before the whole schema is checked against an exact contract and reparsed: - the default is discovered after, or before, a default-less use; - uses through a direct field, array and map defaults, and a record member; - an independent nullable envelope with a null default. Controls: several compatible values, no retained default, reverse canonical order, and same short name under different full names (non-strict keeps the existing -0/-1 mapping). Incompatible projected bodies of one source name must be rejected by the converter, naming the type and both fields, not by Avro's "Can't redefine". Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…CTIONITEM-25474)
F8-T1 (clarifications section 12a): with f8neg_r canonical, v_f8neg_r must
succeed in both modes with the canonical output types A{x} and B{y}. Compare A
and B in memory against independent contracts before serialization, assert no
Shared survives and no default is added, then compare and reparse the whole
schema, repeat the conversion, and check the sources are unchanged.
F8-T2: the forward rejection must name a qualified Shared identity under the
existing mapping (non-strict: source, pre-merge or final name), not just the
short name, besides both field paths.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ts (ACTIONITEM-25474) Each use of a named record was ordered on its own by the defaults retained at that use, so a default-less use and a use with a default could carry different bodies under one full name and the result could not be serialized. A new SharedNamedDefinitions pass over the finished merge result groups uses by the output full name, collects the retained default demands of all uses, selects the one nullable-option order that satisfies them, and relinks every use to one rebuilt body (fresh fields, no source mutation). Uses that cannot be the same definition, or demands that contradict, are rejected with the full name and both paths. Names, namespaces, defaults and metadata are unchanged; docs and aliases of the first use win. The toString equality shortcut in the merge is removed so that a non-canonical branch with differently projected uses is never serialized before the canonical names are chosen. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ITEM-25474)
A standalone root Project is itself the public conversion result, so retained
incompatible bodies of one named record must be rejected by the converter
(qualified name and both uses), not left to fail at serialization. Covers
projected {x}/{y} and projected {x}/pass-through {x,y} uses, a compatible
{x}/{x} control, and the same Project as a non-canonical UNION input under
canonical f8neg_r, which must not be rejected eagerly.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…TIONITEM-25474) SharedNamedDefinitions only ran while merging UNION inputs, so a standalone projection that retained incompatible bodies of one named record was returned unserializable. RelToAvroSchemaConverter.convert now settles shared definitions on the result it returns; intermediate branches stay occurrence-local so UNION inputs can still take the canonical names. A recursive input is passed through unchanged (recursion is not newly supported). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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 are proposed in this pull request, and why are they necessary?
Bug. A fuzzy UNION lines branch fields up case-insensitively and wraps the evolved branch in
generic_project(...). Avro inference (RelToAvroSchemaConverter) rebuilt that projection from lowercase Calcite types. The directly referenced branch kept the source camelCase Avro schema, andSchemaUtilities.mergeUnionRecordSchemamerges with exact case. So valid views failed with "Found two incompatible schemas for LogicalUnion operator", or lost field casing and metadata.Design
AvroSchemaProjection. Recursive projection that keeps metadata: it derives the projected Avro schema from the operand's actual Avro schema instead of regenerating it from the Calcite type.RelToAvroSchemaConverter.generic_projectoperand in isolation and resolves projection-source field access segment by segment.LogicalProjectkeeps input record metadata.SchemaUtilities.SharedNamedDefinitions.left.toString()==right.toString()shortcut inmergeRecordswas removed. It could serialize a non-canonical branch before canonical names were chosen.VARBINARY(n)projection targets. Treated as a maximum: they reject an Avrofixed(N)withn < Nand reject unboundedbytes. Annotated non-decimal fixed proves its own size.Compatibility-visible changes (please review)
nulldefaults now survive copies and normalization.VARBINARY(n)rejection.fixed(N)(n < N) or ofbytes.generic_projectflow produces a finite VARBINARY target, and first-branch VARBINARY leaves already fail inRelDataTypeToAvroType.SharedNamedDefinitionscontains an inertRecursiveSchemaExceptioncatch, but nothing throws it, so recursive inputs are not passed through by that path.Known limitations
How was this patch tested?
New and updated unit tests in
coral-schema:FuzzyUnionAvroSchemaTests,FuzzyUnionStructuralTests,FuzzyUnionAliasTestsandFuzzyUnionSharedDefinitionTests.ViewToAvroSchemaConverterTestsandSchemaUtilitiesTestsgoldens.coral-schema/src/test/resources/fuzzyunion/.The tests cover both strict modes, both branch orders, nested records/arrays/maps/unions, defaults, aliases, decimal/fixed/binary parameters, shared named definitions, and negative controls.
Commands, run with JDK 17:
./gradlew :coral-schema:cleanTest :coral-schema:test— passed, 309/309, 0 skipped../gradlew :coral-hive:cleanTest :coral-hive:test— passed (includesFuzzyUnionTest13/13)../gradlew :coral-spark:cleanTest :coral-spark:test— passed (includesFuzzyUnionViewTest11/11)../gradlew :coral-schema:spotlessJavaCheck— passed. It was run in a temporary clone because of a worktree JGit limitation.Not run:
:coral-spark:spark3Testcoral-spark-catalogtests./gradlew clean build