Skip to content

feat: support catalog provider in datafusion#176

Merged
JingsongLi merged 2 commits intoapache:mainfrom
luoyuxia:support-catalog-provider
Apr 2, 2026
Merged

feat: support catalog provider in datafusion#176
JingsongLi merged 2 commits intoapache:mainfrom
luoyuxia:support-catalog-provider

Conversation

@luoyuxia
Copy link
Copy Markdown
Contributor

@luoyuxia luoyuxia commented Apr 1, 2026

Purpose

Linked issue: close #xxx
as a part of #173

Brief change log

Tests

API and Format

Documentation

@JingsongLi
Copy link
Copy Markdown
Contributor

Please rebase master.

@luoyuxia luoyuxia force-pushed the support-catalog-provider branch from c12190f to 14e44fa Compare April 2, 2026 01:47
@luoyuxia
Copy link
Copy Markdown
Contributor Author

luoyuxia commented Apr 2, 2026

Please rebase master.

@JingsongLi Done

Copy link
Copy Markdown
Contributor

@QuakeWang QuakeWang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luoyuxia Left minor comments. PTAL

})
}

fn schema(&self, name: &str) -> Option<Arc<dyn SchemaProvider>> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema() currently returns Some(...) for any name, which makes a non-existent database look like a resolvable schema. On the DataFusion side, the expected behavior is to return None when the schema does not exist. Otherwise, downstream code ends up turning “missing schema” into “table not found” or an empty schema.

I reproduced this locally with a missing database. It would be better to check whether the database exists first and return None if it does not.

Copy link
Copy Markdown
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@JingsongLi JingsongLi merged commit 6be17e8 into apache:main Apr 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants