Skip to content

Commit 935382f

Browse files
authored
perf: Optimize DFSchema::qualified_name (#21722)
## Which issue does this PR close? - Closes #21723 ## Rationale for this change `DFSchema::qualified_name` shows up as a hotspot when profiling the planner. It currently goes through the `fmt` machinery, which is fairly slow; it is reasonably simple to avoid that and `write_str()` to a preallocated string. On the `sql_planner` benchmark, this yields 1-5% improvements for most benchmarks, with no significant regressions. ## What changes are included in this PR? * Implement optimization * Add unit test to ensure that we preserve compatibility between `qualified_name` and `TableReference::Display` ## Are these changes tested? Yes. ## Are there any user-facing changes? No.
1 parent 8614308 commit 935382f

File tree

1 file changed

+67
-4
lines changed

1 file changed

+67
-4
lines changed

datafusion/common/src/dfschema.rs

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,11 +1338,44 @@ impl SchemaExt for Schema {
13381338
}
13391339
}
13401340

1341+
/// Build a fully-qualified field name string. This is equivalent to
1342+
/// `format!("{q}.{name}")` when `qualifier` is `Some`, or just `name` when
1343+
/// `None`. We avoid going through the `fmt` machinery for performance reasons.
13411344
pub fn qualified_name(qualifier: Option<&TableReference>, name: &str) -> String {
1342-
match qualifier {
1343-
Some(q) => format!("{q}.{name}"),
1344-
None => name.to_string(),
1345-
}
1345+
let qualifier = match qualifier {
1346+
None => return name.to_string(),
1347+
Some(q) => q,
1348+
};
1349+
let (first, second, third) = match qualifier {
1350+
TableReference::Bare { table } => (table.as_ref(), None, None),
1351+
TableReference::Partial { schema, table } => {
1352+
(schema.as_ref(), Some(table.as_ref()), None)
1353+
}
1354+
TableReference::Full {
1355+
catalog,
1356+
schema,
1357+
table,
1358+
} => (
1359+
catalog.as_ref(),
1360+
Some(schema.as_ref()),
1361+
Some(table.as_ref()),
1362+
),
1363+
};
1364+
1365+
let extra = second.map_or(0, str::len) + third.map_or(0, str::len);
1366+
let mut s = String::with_capacity(first.len() + extra + 3 + name.len());
1367+
s.push_str(first);
1368+
if let Some(second) = second {
1369+
s.push('.');
1370+
s.push_str(second);
1371+
}
1372+
if let Some(third) = third {
1373+
s.push('.');
1374+
s.push_str(third);
1375+
}
1376+
s.push('.');
1377+
s.push_str(name);
1378+
s
13461379
}
13471380

13481381
#[cfg(test)]
@@ -1351,6 +1384,36 @@ mod tests {
13511384

13521385
use super::*;
13531386

1387+
/// `qualified_name` doesn't use `TableReference::Display` for performance
1388+
/// reasons, but check that the output is consistent.
1389+
#[test]
1390+
fn qualified_name_agrees_with_display() {
1391+
let cases: &[(Option<TableReference>, &str)] = &[
1392+
(None, "col"),
1393+
(Some(TableReference::bare("t")), "c0"),
1394+
(Some(TableReference::partial("s", "t")), "c0"),
1395+
(Some(TableReference::full("c", "s", "t")), "c0"),
1396+
(Some(TableReference::bare("mytable")), "some_column_name"),
1397+
// Empty segments must be preserved so that distinct qualified
1398+
// fields don't collide in `DFSchema::field_names()`.
1399+
(Some(TableReference::bare("")), "col"),
1400+
(Some(TableReference::partial("s", "")), "col"),
1401+
(Some(TableReference::partial("", "t")), "col"),
1402+
(Some(TableReference::full("c", "", "t")), "col"),
1403+
(Some(TableReference::full("", "s", "t")), "col"),
1404+
(Some(TableReference::full("c", "s", "")), "col"),
1405+
(Some(TableReference::full("", "", "")), "col"),
1406+
];
1407+
for (qualifier, name) in cases {
1408+
let actual = qualified_name(qualifier.as_ref(), name);
1409+
let expected = match qualifier {
1410+
Some(q) => format!("{q}.{name}"),
1411+
None => name.to_string(),
1412+
};
1413+
assert_eq!(actual, expected, "qualifier={qualifier:?} name={name}");
1414+
}
1415+
}
1416+
13541417
#[test]
13551418
fn qualifier_in_name() -> Result<()> {
13561419
let col = Column::from_name("t1.c0");

0 commit comments

Comments
 (0)