Skip to content

refactor(catalog): unify name resolution with SchemaResolver trait #1492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leoyvens wants to merge 6 commits into
base: main
Choose a base branch
from
Open

refactor(catalog): unify name resolution with SchemaResolver trait #1492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54f4a2a
refactor(catalog): unify name resolution with SchemaResolver trait
leoyvens Dec 18, 2025
3c85561
style: fix rustfmt import ordering
leoyvens Dec 18, 2025
e78a620
fix: remove needless borrow for clippy 1.92
leoyvens Dec 19, 2025
68f889c
Rename DynamicResolver to RegistrySchemaResolver
leoyvens Dec 19, 2025
3ebf6cb
Move resolve_logical_catalog to top of resolve.rs
leoyvens Dec 19, 2025
cf58767
Remove {0} from error messages with #[source]
leoyvens Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/core/common/src/catalog.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ pub mod errors;
pub mod logical;
pub mod physical;
pub mod reader;
pub mod resolve;
pub mod sql;
2 changes: 1 addition & 1 deletion crates/core/common/src/catalog/dataset_access.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{BoxError, Dataset};
///
/// This trait provides the minimal interface required for SQL catalog building,
/// abstracting over the dataset store implementation.
pub trait DatasetAccess {
pub trait DatasetAccess: Send + Sync + 'static {
Copy link
Contributor

@LNSD LNSD Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Are these necessary? Can't we just require them in the where clause, or let the compiler infer them?

Copy link
Collaborator Author

@leoyvens leoyvens Dec 19, 2025

Choose a reason for hiding this comment

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

We can put them in the where clause, but I have the habit of doing this for syntactical convenience. It's an internal trait so we can change whenever.

/// Resolve a dataset reference to a hash reference.
///
/// This method resolves a dataset reference (which may contain a version, "latest", etc.)
Expand Down
16 changes: 14 additions & 2 deletions crates/core/common/src/catalog/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ pub enum CatalogForSqlError {
/// exists in the dataset definition but has no physical parquet files.
#[error("Table '{table}' has not been synced")]
TableNotSynced { table: String },

/// Failed during catalog resolution.
///
/// This wraps errors from the core resolution logic.
#[error("Catalog resolution failed")]
Resolution(#[source] BoxError),
}

impl CatalogForSqlError {
Expand Down Expand Up @@ -174,11 +180,17 @@ pub enum PlanningCtxForSqlError {
/// dataset does not support eth_call (not an EVM RPC dataset or no provider configured).
#[error("Function 'eth_call' not available for dataset '{reference}'")]
EthCallNotAvailable { reference: HashReference },

/// Failed during catalog resolution.
///
/// This wraps errors from the core resolution logic.
#[error("Catalog resolution failed")]
Resolution(#[source] BoxError),
}

/// Errors specific to catalog_for_sql_with_deps operations
/// Errors specific to catalog_for_derived_table operations
///
/// This error type is used exclusively by `catalog_for_sql_with_deps()` to create
/// This error type is used exclusively by `catalog_for_derived_table()` to create
/// a physical catalog for SQL query execution with pre-resolved dependencies.
#[derive(Debug, thiserror::Error)]
#[allow(clippy::large_enum_variant)]
Expand Down
Loading