Skip to content

Commit 856d417

Browse files
authored
fix(datafusion): avoid loading tables for SHOW TABLES (#576)
1 parent 6824972 commit 856d417

2 files changed

Lines changed: 190 additions & 9 deletions

File tree

crates/integrations/datafusion/src/catalog.rs

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use std::sync::RwLock;
2525
use async_trait::async_trait;
2626
use datafusion::catalog::{CatalogProvider, MemorySchemaProvider, SchemaProvider};
2727
use datafusion::common::{plan_datafusion_err, Column};
28-
use datafusion::datasource::TableProvider;
28+
use datafusion::datasource::{TableProvider, TableType};
2929
use datafusion::error::Result as DFResult;
3030
use datafusion::execution::SessionState;
3131
use datafusion::logical_expr::{expr_fn::cast, Expr, LogicalPlan, LogicalPlanBuilder};
@@ -338,6 +338,8 @@ pub struct PaimonSchemaProvider {
338338
dynamic_options: DynamicOptions,
339339
/// Optional temporary in-memory provider for temp tables and views.
340340
temp_provider: Option<Arc<MemorySchemaProvider>>,
341+
/// Table types populated together with `table_names` for metadata-only lookups.
342+
catalog_table_types: RwLock<HashMap<String, TableType>>,
341343
blob_reader_registry: BlobReaderRegistry,
342344
session_state: Option<SessionStateProvider>,
343345
schema_force_view_types: bool,
@@ -369,6 +371,7 @@ impl PaimonSchemaProvider {
369371
database,
370372
dynamic_options,
371373
temp_provider,
374+
catalog_table_types: RwLock::new(HashMap::new()),
372375
blob_reader_registry,
373376
session_state,
374377
schema_force_view_types: true,
@@ -386,30 +389,46 @@ impl SchemaProvider for PaimonSchemaProvider {
386389
fn table_names(&self) -> Vec<String> {
387390
let catalog = Arc::clone(&self.catalog);
388391
let database = self.database.clone();
389-
let mut names = block_on_with_runtime(
392+
let (mut names, views) = block_on_with_runtime(
390393
{
391394
let db = database.clone();
392395
async move {
393-
let mut names = match catalog.list_tables(&db).await {
396+
let names = match catalog.list_tables(&db).await {
394397
Ok(names) => names,
395398
Err(e) => {
396399
log::error!("failed to list tables in '{}': {e}", db);
397400
vec![]
398401
}
399402
};
400-
match catalog.list_views(&db).await {
401-
Ok(views) => names.extend(views),
402-
Err(paimon::Error::Unsupported { .. }) => {}
403+
let views = match catalog.list_views(&db).await {
404+
Ok(views) => views,
405+
Err(paimon::Error::Unsupported { .. }) => vec![],
403406
Err(error) => {
404407
log::error!("failed to list views in '{}': {error}", db);
408+
vec![]
405409
}
406-
}
407-
names
410+
};
411+
(names, views)
408412
}
409413
},
410414
"paimon catalog access thread panicked",
411415
);
412416

417+
let mut catalog_table_types = HashMap::with_capacity(names.len() + views.len());
418+
for name in &names {
419+
catalog_table_types.insert(name.clone(), TableType::Base);
420+
}
421+
for view in views {
422+
catalog_table_types
423+
.entry(view.clone())
424+
.or_insert(TableType::View);
425+
names.push(view);
426+
}
427+
*self
428+
.catalog_table_types
429+
.write()
430+
.unwrap_or_else(|e| e.into_inner()) = catalog_table_types;
431+
413432
if let Some(temp) = &self.temp_provider {
414433
names.extend(temp.table_names());
415434
}
@@ -540,6 +559,27 @@ impl SchemaProvider for PaimonSchemaProvider {
540559
.await
541560
}
542561

562+
async fn table_type(&self, name: &str) -> DFResult<Option<TableType>> {
563+
if let Some(temp) = &self.temp_provider {
564+
if let Some(table_type) = temp.table_type(name).await? {
565+
return Ok(Some(table_type));
566+
}
567+
}
568+
569+
if let Some(table_type) = self
570+
.catalog_table_types
571+
.read()
572+
.unwrap_or_else(|e| e.into_inner())
573+
.get(name)
574+
{
575+
return Ok(Some(*table_type));
576+
}
577+
578+
self.table(name)
579+
.await
580+
.map(|table| table.map(|table| table.table_type()))
581+
}
582+
543583
fn table_exist(&self, name: &str) -> bool {
544584
if let Some(temp) = &self.temp_provider {
545585
if temp.table_exist(name) {

crates/integrations/datafusion/tests/sql_context_tests.rs

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
//! SQL context integration tests for paimon-datafusion.
1919
20-
use std::sync::atomic::{AtomicBool, Ordering};
20+
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
2121
use std::sync::{Arc, Mutex};
2222

2323
use async_trait::async_trait;
@@ -50,6 +50,105 @@ async fn create_sql_context(catalog: Arc<FileSystemCatalog>) -> SQLContext {
5050
ctx
5151
}
5252

53+
struct MetadataListingCatalog {
54+
get_table_calls: AtomicUsize,
55+
}
56+
57+
impl MetadataListingCatalog {
58+
fn new() -> Self {
59+
Self {
60+
get_table_calls: AtomicUsize::new(0),
61+
}
62+
}
63+
64+
fn get_table_calls(&self) -> usize {
65+
self.get_table_calls.load(Ordering::SeqCst)
66+
}
67+
}
68+
69+
#[async_trait]
70+
impl Catalog for MetadataListingCatalog {
71+
async fn list_databases(&self) -> paimon::Result<Vec<String>> {
72+
Ok(vec!["default".to_string()])
73+
}
74+
75+
async fn create_database(
76+
&self,
77+
_name: &str,
78+
_ignore_if_exists: bool,
79+
_properties: std::collections::HashMap<String, String>,
80+
) -> paimon::Result<()> {
81+
Ok(())
82+
}
83+
84+
async fn get_database(&self, name: &str) -> paimon::Result<paimon::catalog::Database> {
85+
Ok(paimon::catalog::Database::new(
86+
name.to_string(),
87+
std::collections::HashMap::new(),
88+
None,
89+
))
90+
}
91+
92+
async fn drop_database(
93+
&self,
94+
_name: &str,
95+
_ignore_if_not_exists: bool,
96+
_cascade: bool,
97+
) -> paimon::Result<()> {
98+
Ok(())
99+
}
100+
101+
async fn get_table(&self, _identifier: &Identifier) -> paimon::Result<paimon::table::Table> {
102+
self.get_table_calls.fetch_add(1, Ordering::SeqCst);
103+
Err(paimon::Error::Unsupported {
104+
message: "table loading is unavailable".to_string(),
105+
})
106+
}
107+
108+
async fn list_tables(&self, _database_name: &str) -> paimon::Result<Vec<String>> {
109+
Ok(vec!["metadata_only".to_string()])
110+
}
111+
112+
async fn list_views(&self, _database_name: &str) -> paimon::Result<Vec<String>> {
113+
Ok(vec!["metadata_view".to_string()])
114+
}
115+
116+
async fn create_table(
117+
&self,
118+
_identifier: &Identifier,
119+
_creation: paimon::spec::Schema,
120+
_ignore_if_exists: bool,
121+
) -> paimon::Result<()> {
122+
Ok(())
123+
}
124+
125+
async fn drop_table(
126+
&self,
127+
_identifier: &Identifier,
128+
_ignore_if_not_exists: bool,
129+
) -> paimon::Result<()> {
130+
Ok(())
131+
}
132+
133+
async fn rename_table(
134+
&self,
135+
_from: &Identifier,
136+
_to: &Identifier,
137+
_ignore_if_not_exists: bool,
138+
) -> paimon::Result<()> {
139+
Ok(())
140+
}
141+
142+
async fn alter_table(
143+
&self,
144+
_identifier: &Identifier,
145+
_changes: Vec<SchemaChange>,
146+
_ignore_if_not_exists: bool,
147+
) -> paimon::Result<()> {
148+
Ok(())
149+
}
150+
}
151+
53152
struct PartitionCatalog {
54153
inner: Arc<FileSystemCatalog>,
55154
fail_list_partitions: AtomicBool,
@@ -265,6 +364,48 @@ async fn test_show_tables_is_enabled() {
265364
.expect("SHOW TABLES should execute");
266365
}
267366

367+
#[tokio::test]
368+
async fn test_show_tables_does_not_load_table_providers() {
369+
let catalog = Arc::new(MetadataListingCatalog::new());
370+
let mut sql_context = SQLContext::new();
371+
sql_context
372+
.register_catalog("paimon", catalog.clone())
373+
.await
374+
.unwrap();
375+
376+
let table_names = collect_string_column(&sql_context, "SHOW TABLES", "table_name").await;
377+
378+
assert!(table_names.contains(&"metadata_only".to_string()));
379+
assert_eq!(catalog.get_table_calls(), 0);
380+
381+
assert!(sql_context
382+
.sql("SELECT * FROM metadata_only")
383+
.await
384+
.is_err());
385+
assert!(catalog.get_table_calls() > 0);
386+
}
387+
388+
#[tokio::test]
389+
async fn test_show_tables_preserves_catalog_view_type() {
390+
let catalog = Arc::new(MetadataListingCatalog::new());
391+
let mut sql_context = SQLContext::new();
392+
sql_context
393+
.register_catalog("paimon", catalog.clone())
394+
.await
395+
.unwrap();
396+
397+
let table_types = collect_string_column(
398+
&sql_context,
399+
"SELECT table_type FROM information_schema.tables \
400+
WHERE table_schema = 'default' AND table_name = 'metadata_view'",
401+
"table_type",
402+
)
403+
.await;
404+
405+
assert_eq!(table_types, vec!["VIEW"]);
406+
assert_eq!(catalog.get_table_calls(), 0);
407+
}
408+
268409
#[tokio::test]
269410
async fn test_select_branch_table_reads_branch_snapshot() {
270411
let (_tmp, catalog) = create_test_env();

0 commit comments

Comments
 (0)