Skip to content

fix allOf of oneOfs by introducing cartesian product #982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PolyProgrammist wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix allOf of oneOfs by introducing cartesian product #982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
efe34d3
fix allOf of oneOfs by introducing cartesian product
PolyProgrammist Dec 4, 2025
7674d44
check schemas partial eq
PolyProgrammist Mar 6, 2026
c0cca79
roughly
PolyProgrammist Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion typify-impl/src/merge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,20 @@ pub(crate) fn try_merge_with_subschemas(
}

if let Some(one_of) = one_of {
let merged_subschemas = try_merge_with_each_subschema(&schema_object, one_of, defs);
// Check if the base schema already has a oneOf - if so, we need to
// compute the Cartesian product of the two oneOfs
let base_one_of = schema_object
.subschemas
.as_ref()
.and_then(|ss| ss.one_of.as_ref());

let merged_subschemas = if let Some(base_variants) = base_one_of {
// Cartesian product: for each base variant and each new variant,
// merge them together
try_merge_oneof_cartesian_product(base_variants, one_of, defs)
} else {
try_merge_with_each_subschema(&schema_object, one_of, defs)
};

match merged_subschemas.len() {
0 => return Err(()),
Expand Down Expand Up @@ -413,6 +426,34 @@ fn try_merge_with_each_subschema(
joined_schemas
}

/// Compute the Cartesian product of two oneOf schemas. For each combination
/// of a variant from the first oneOf and a variant from the second oneOf,
/// merge them together. This is used when we have `allOf: [oneOf[A,B], oneOf[C,D]]`
/// which should produce `oneOf[A∩C, A∩D, B∩C, B∩D]`.
fn try_merge_oneof_cartesian_product(
base_variants: &[Schema],
new_variants: &[Schema],
defs: &BTreeMap<RefKey, Schema>,
) -> Vec<Schema> {
let mut result = Vec::new();
Copy link
Collaborator

@ahl ahl Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems possible for there to be literal or effective duplicates in this list.

Copy link
Author

@PolyProgrammist PolyProgrammist Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there may be oneOf: [A, B] and oneOf: [C, D]
And maybe something like oneOf[{}, {type: string}] and oneOf[{type:string}, {type:string}] where for all of them the result would be {type:string}.

Do you think comparing like that is enough?

Copy link
Collaborator

@ahl ahl Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there may be oneOf: [A, B] and oneOf: [C, D] And maybe something like oneOf[{}, {type: string}] and oneOf[{type:string}, {type:string}] where for all of them the result would be {type:string}.

That's not quite right: oneOf[{type: string}, {type: string}] would be equivalent to the schema false... and so therefore would the outer allOf.

but perhaps something like this?

{
  "allOf": [
    {
      "oneOf": [
        {
          "type": "object",
          "required": ["x"],
          "properties": {
            "x": { "const": 1 }
          }
        },
        {
          "type": "object",
          "required": ["x"],
          "properties": {
            "x": { "const": 2 }
          }
        }
      ]
    },
    {
      "oneOf": [
        {
          "type": "object",
          "required": ["y"],
          "properties": {
            "y": { "const": 3 }
          }
        },
        {
          "type": "object",
          "required": ["x", "y"],
          "properties": {
            "x": { "const": 1 },
            "y": { "const": 3 }
          }
        }
      ]
    }
  ]
}

I think the expansion you have in mind is more of less

{
  "oneOf": [
    {
      "allOf": [
        { "type": "object", "required": ["x"], "properties": { "x": { "const": 1 } } },
        { "type": "object", "required": ["y"], "properties": { "y": { "const": 3 } } }
      ]
    },
    {
      "allOf": [
        { "type": "object", "required": ["x"], "properties": { "x": { "const": 1 } } },
        { "type": "object", "required": ["x", "y"], "properties": { "x": { "const": 1 }, "y": { "const": 3 } } }
      ]
    },
    {
      "allOf": [
        { "type": "object", "required": ["x"], "properties": { "x": { "const": 2 } } },
        { "type": "object", "required": ["y"], "properties": { "y": { "const": 3 } } }
      ]
    },
    {
      "allOf": [
        { "type": "object", "required": ["x"], "properties": { "x": { "const": 2 } } },
        { "type": "object", "required": ["x", "y"], "properties": { "x": { "const": 1 }, "y": { "const": 3 } } }
      ]
    }
  ]
}

And if we further reduce that we get

{
  "oneOf": [
    {
      "type": "object",
      "required": ["x", "y"],
      "properties": {
        "x": { "const": 1 },
        "y": { "const": 3 }
      }
    },
    {
      "type": "object",
      "required": ["x", "y"],
      "properties": {
        "x": { "const": 1 },
        "y": { "const": 3 }
      }
    },
    {
      "type": "object",
      "required": ["x", "y"],
      "properties": {
        "x": { "const": 2 },
        "y": { "const": 3 }
      }
    },
    false
  ]
}

The first 2 are identical; the last one is unsatisfiable. I think we need check for exclusivity--perhaps by seeing if the A ∪ ⌐B is empty?

Copy link
Collaborator

@ahl ahl Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually that's not quite right and now I'm deeply uncertain about whether there might be duplicates.

Copy link
Collaborator

@ahl ahl Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I've had time to sleep on it:

If we have two sets of schemas, each known to consist of mutually exclusive schemas, then the cartesian product of those two sets must also be pairwise mutually exclusive. Why? A value that was valid for multiple schemas of the cartesian product would necessarily be valid for multiple schemas in (at least) one of the input schema sets (which would invalidate the initial premise).

However! I don't believe that typify currently ensures that oneOfs are mutually exclusive. Part of the (in-progress) typify rewrite is to ensure that that mutual exclusivity. For example, with an input like:

{
      "oneOf": [
        {
          "type": "object",
          "required": ["y"],
          "properties": {
            "y": { "const": 3 }
          }
        },
        {
          "type": "object",
          "required": ["x", "y"],
          "properties": {
            "x": { "const": 1 },
            "y": { "const": 3 }
          }
        }
      ]
    }

We would produce a type like this:

pub struct MyType {
    y: MyTypeY, // unit type that can only have a value of the integer 3
    x: MyTypeX, // basically serde_json::Value except *not* the integer 1
}

Which is all to say: this PR is wrong, but it's no more wrong that what we currently have and, indeed, it's quite a bit better in all but the most esoteric cases.


for base_variant in base_variants {
for new_variant in new_variants {
// Try to merge each pair of variants
if let Ok(merged) = try_merge_schema(base_variant, new_variant, defs) {
// Only include if the merge produced a satisfiable schema
if merged != Schema::Bool(false)
Copy link
Collaborator

@ahl ahl Mar 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does try_merge_schema return false ever? Perhaps we should use merge_schema instead?

&& !result.iter().any(|existing| existing.roughly(&merged))
{
result.push(merged);
}
}
}
}

result
}

fn merge_schema_not(
schema: &Schema,
not_schema: &Schema,
Expand Down
Loading