Skip to content

PhysicalExprAdapter should use SchemaAdapter logic for casting structs #18957

Closed

Description

@a10y

Describe the bug

If we try and rewrite an expression that operates over a struct column, currently the DefaultPhysicalExprRewriter will fail b/c the arrow cast kernel uses an overly strict check for Struct castability, namely that the two structs have identical types.

let is_compatible =
can_cast_types(physical_field.data_type(), logical_field.data_type());

However, the DefaultSchemaAdapter does the right thing here, checking for backward-compatible schema compatibility using the can_cast_field function

(DataType::Struct(source_fields), DataType::Struct(target_fields)) => {
// validate_struct_compatibility returns Result<()>; on success we can cast structs
validate_struct_compatibility(source_fields, target_fields)?;
Ok(true)
}

To Reproduce

No response

Expected behavior

No response

Additional context

It's possible this is addressed by #18627, haven't had a chance to look through it yet.

Activity

  1. added
    bugSomething isn't working
    on Nov 26, 2025
  2. added theissue type on Nov 26, 2025
  3. jatinkumarsingh commented on Nov 27, 2025

    @jatinkumarsingh
    Contributor

    take

  4. jatinkumarsingh commented on Nov 27, 2025

    @jatinkumarsingh
    Contributor

    @a10y
    would love to work on this issue
    plz asssign me this
    thnku

  5. adriangb commented on Nov 27, 2025

    @adriangb
    Contributor

    I don't think using SchemaAdapter is the right move. Instead I think we need to extract the logic from it into a generalized cast operator that does the right thing and can be used from PhysicalExprAdapter. Even outside of this context it would be nice to be able to do cast(struct_col, struct_type) in sql, etc. As per #16800 the plan is to essentially get rid of SchemaAdapter long term.

  6. brancz commented on Nov 27, 2025

    @brancz
    Contributor

    Even outside of this context it would be nice to be able to do cast(struct_col, struct_type) in sql, etc.

    Is it? I would argue it's too easy to accidentally use that API incorrectly and cast will silently backfill nulls when really all you've done is a typo (for example).

    I actually think this is better off in the adapter/scan area.

  7. adriangb commented on Nov 27, 2025

    @adriangb
    Contributor

    I think it’s reasonable to want to do. And either way for it to go in the adapter it has to be an expression, and I think getting it in the adapter is the important part.

  8. adriangb commented on Dec 7, 2025

    @adriangb
    Contributor
  9. a10y commented on Dec 14, 2025

    @a10y
    ContributorAuthor

    I can confirm from Vortex land that this got fixed in #18998 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions