Possible reproducer of schema metadata bug.

[wiedld]

Which issue does this PR close?

Related to #12560

Rationale for this change

A PR merged (and released in 42.0.0) has added a check that the logical and physical plan input schemas (to an aggregate node) are the same.

Once released into the wild, it began being triggered. One example case is where field metadata is in the logical plan, but missing from the physical plan.

I haven't yet re-created that exact reproducer. However, I ended up creating the inverse case -- where the metadata is in the physical plan, but missing from the logical plan.

What changes are included in this PR?

First commit: changed the From DFSchema for SchemaRef, so it stops dropping the metadata.

Second commit: demonstrate that test cases (where metadata is on the data) end up failing due to the same error as in #12560.

Are these changes tested?

Are there any user-facing changes?

[wiedld]

Check failed with:

physical_input_schema: 
Schema { 
    fields: [
        Field { 
            name: "id", 
            data_type: Int32, 
            nullable: true, 
            dict_id: 0, 
            dict_is_ordered: false, 
            metadata: {"metadata_key": "the id field"} 
        }
    ], 
    metadata: {"metadata_key": "the entire schema"} 
}

physical_input_schema_from_logical: 
Schema { 
    fields: [
        Field { 
            name: "id", 
            data_type: Int32, 
            nullable: true, 
            dict_id: 0, 
            dict_is_ordered: false, 
            metadata: {"metadata_key": "the id field"} 
        }
    ], 
    metadata: {} 
}

[wiedld]

Note: CI is failing due to this change merged today. @akurmustafa

[alamb]

Note: CI is failing due to this change merged today. @akurmustafa

I believe this was fixed by @doupache in #12657. I will retrigger CI

[alamb]

Thanks @wiedld

I am not sure about this particular change as it seems to introduce a bug

Here is an idea of how to try and reproduce the issue we see:

1. Add a new sqllogictest like metadata.rs or something: https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest
2. Add custom / rust setup code here that creates a table that has schemas with metadata (both metadata on the schema and on the fields):

   datafusion/datafusion/sqllogictest/src/test_context.rs

   Lines 104 to 109 in 792f07d

   	"joins.slt" => {
   	info!("Registering partition table tables");
   	let example_udf = create_example_udf();
   	test_ctx.ctx.register_udf(example_udf);
   	register_partition_table(&mut test_ctx).await;
   	}

3. Add various queries in metadata.slt with the appropriate plan nodes (GROUP BY, ORDER BY, etc) and see if you can provoke the same error.

[alamb]

This change effectively discards the metadata when converting from DFSchema --> Schema (HashMap::new() is empty metadata)

So I am not surprised this can trigger the issue described (as it introduces a bug)

[wiedld]

Yeah, this is why I wasn't pushing (other) code yesterday. Knew I was brain fried. Sorry about that.

[alamb]

no worries -- I think we are making progress. I am particularly stoked about the metadata.slt idea to try and trigger some of the errors we are seeing (and I think that would make it easier to avoid this kind of regression going forward)

[alamb]

I filed #12687 to track the metadata lost bug we are seeing

[wiedld]

Have the correct reproducer & fix. Closing this one.