Skip to content

datafusion-proto: column qualifiers containing . are silently corrupted on round-trip #24776

Closed

Description

@JoeryH

Describe the bug

datafusion-proto serializes a column's qualifier as a single unquoted string and re-parses it on the way back:

  • encode — proto-common/src/to_proto/mod.rs:
    relation: c.relation.map(|relation| protobuf::ColumnRelation {
        relation: relation.to_string(),
    }),
  • decode — proto-common/src/from_proto/mod.rs:
    impl From<protobuf::ColumnRelation> for TableReference {
        fn from(rel: protobuf::ColumnRelation) -> Self {
            Self::parse_str_normalized(rel.relation.as_str(), true)
        }
    }

TableReference's Display writes the parts joined by . without quoting, while parse_str_normalized splits on unquoted .. So any TableReference whose segments contain a . does not round-trip. The same applies to DfField.qualifier, which is encoded the same way.

This is silent: no error is raised, the reference is just re-partitioned into different segments. A Partial { schema: "my.schema", table: "t" } comes back as Full { catalog: "my", schema: "schema", table: "t" }, which then resolves against a different (or non-existent) table.

Identifiers containing dots are legal — any dialect that allows quoted identifiers can produce them, and they show up in practice when an external catalog's naming is mapped into DataFusion.

To Reproduce

Through the actual proto conversions (datafusion-proto-common, main @ 8332cfa):

use datafusion_common::{Column, TableReference};
use datafusion_proto_common as protobuf;

fn roundtrip(col: Column) {
    let encoded: protobuf::Column = (&col).into();
    let decoded: Column = (&encoded).into();
    let ok = if decoded == col { "OK  " } else { "LOSS" };
    println!("{ok} {col:?}\n       -> proto relation {:?}\n       -> {decoded:?}\n",
        encoded.relation.as_ref().map(|r| &r.relation));
}

fn main() {
    roundtrip(Column::new(Some(TableReference::full("c", "s", "t")), "x"));
    roundtrip(Column::new(Some(TableReference::partial("my.schema", "t")), "x"));
    roundtrip(Column::new(Some(TableReference::bare("has.dot")), "x"));
}

Output:

OK   Column { relation: Some(Full { catalog: "c", schema: "s", table: "t" }), name: "x" }
       -> proto relation Some("c.s.t")
       -> Column { relation: Some(Full { catalog: "c", schema: "s", table: "t" }), name: "x" }

LOSS Column { relation: Some(Partial { schema: "my.schema", table: "t" }), name: "x" }
       -> proto relation Some("my.schema.t")
       -> Column { relation: Some(Full { catalog: "my", schema: "schema", table: "t" }), name: "x" }

LOSS Column { relation: Some(Bare { table: "has.dot" }), name: "x" }
       -> proto relation Some("has.dot")
       -> Column { relation: Some(Partial { schema: "has", table: "dot" }), name: "x" }

The underlying TableReference round-trip (Display -> parse_str_normalized), which is what the proto layer relies on, fails the same way and also has a case that collapses to a single bare segment:

LOSS Full { catalog: "cat", schema: "my.schema", table: "t" }
       -> "cat.my.schema.t"
       -> Bare { table: "cat.my.schema.t" }

LOSS Partial { schema: "s", table: "tbl.with.dots" }
       -> "s.tbl.with.dots"
       -> Bare { table: "s.tbl.with.dots" }

Expected behavior

A Column/DfField qualifier round-trips through datafusion-proto unchanged, whatever characters its segments contain.

Additional context

Two possible fixes:

  1. Quote on the way out. Encode with TableReference::to_quoted_string() and decode with a parser that honours the quoting. This is a wire-compatible change in the sense that the proto schema is untouched, but blobs written by older versions still decode by the old rules, so it is only correct if both ends move together.

  2. Make ColumnRelation structured, e.g. adding repeated string parts = 2; alongside the existing string relation = 1;. New writers populate both, readers prefer parts when present. This is properly backward and forward compatible and removes the parse round-trip entirely.

I'd lean towards (2), since it also removes the ambiguity for readers of blobs written by older versions. Happy to put up a PR for whichever the maintainers prefer.

Activity

  1. AnuragRaut08 commented on Aug 31, 2026

    @AnuragRaut08
    Contributor

    I’d be interested in taking on fix (2), making ColumnRelation structured to preserve qualifier segments without relying on string parsing. I can investigate the protobuf compatibility and implement the round-trip tests along with the fix.

  2. JoeryH commented on Aug 31, 2026

    @JoeryH
    Author

    I’d be interested in taking on fix (2), making ColumnRelation structured to preserve qualifier segments without relying on string parsing. I can investigate the protobuf compatibility and implement the round-trip tests along with the fix.

    @AnuragRaut08 I'm happy with whatever!

  3. AnuragRaut08 commented on Sep 15, 2026

    @AnuragRaut08
    Contributor

    Implemented fix (2) in #25334.

    Added parts to ColumnRelation to preserve qualifier boundaries, while keeping relation for backward compatibility. Added round-trip tests for dotted table/schema names and a legacy decode test.

    This avoids relying on parsing the dotted string during deserialization.

  4. AnuragRaut08 commented on Oct 3, 2026

    @AnuragRaut08
    Contributor

    Hey @kosiew, whenever you get a chance, could you please review this PR#25334 Would really appreciate your feedback. If possible, could you also tag a relevant reviewer? Thanks!

  5. added a commit that references this issue on Oct 7, 2026
    7b1e55d
  6. added a commit that references this issue on Oct 11, 2026
    7b59952
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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions