Skip to content

Commit 410fac5

Browse files
beinanclaude
andauthored
refactor: remove catalog re-export modules (#131)
## Summary Addresses code review feedback from PR #130 by removing redundant re-export modules and updating imports to use `lance_graph_catalog` directly. ## Changes - Removed `crates/lance-graph/src/namespace/` directory (only contained re-exports) - Removed `crates/lance-graph/src/source_catalog.rs` (only contained re-exports) - Updated `lib.rs` to re-export catalog types directly from `lance_graph_catalog` - Updated all internal imports in datafusion_planner, query, and Python bindings to use `lance_graph_catalog` ## Test plan - [x] `cargo check --all` passes - [x] `cargo test --all` passes (all 4 doc tests pass, all unit tests pass) Resolves review comments from #130 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b6e29a7 commit 410fac5

12 files changed

Lines changed: 24 additions & 35 deletions

File tree

crates/lance-graph/src/datafusion_planner/config_helpers.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use super::analysis::PlanningContext;
1010
use super::DataFusionPlanner;
1111
use crate::config::{NodeMapping, RelationshipMapping};
1212
use crate::error::Result;
13-
use crate::source_catalog::GraphSourceCatalog;
13+
use lance_graph_catalog::GraphSourceCatalog;
1414
use std::sync::Arc;
1515

1616
impl DataFusionPlanner {

crates/lance-graph/src/datafusion_planner/join_ops.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ use crate::ast::{PropertyValue, RelationshipDirection};
1414
use crate::case_insensitive::qualify_column;
1515
use crate::config::{NodeMapping, RelationshipMapping};
1616
use crate::error::Result;
17-
use crate::source_catalog::GraphSourceCatalog;
1817
use datafusion::logical_expr::{
1918
col, BinaryExpr, Expr, JoinType, LogicalPlan, LogicalPlanBuilder, Operator, TableSource,
2019
};
20+
use lance_graph_catalog::GraphSourceCatalog;
2121
use std::collections::HashMap;
2222
use std::sync::Arc;
2323

crates/lance-graph/src/datafusion_planner/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ pub use analysis::{PlanningContext, QueryAnalysis, RelationshipInstance};
3232
use crate::config::GraphConfig;
3333
use crate::error::Result;
3434
use crate::logical_plan::LogicalOperator;
35-
use crate::source_catalog::GraphSourceCatalog;
3635
use datafusion::logical_expr::LogicalPlan;
36+
use lance_graph_catalog::GraphSourceCatalog;
3737
use std::sync::Arc;
3838

3939
/// Planner abstraction for graph-to-physical planning

crates/lance-graph/src/datafusion_planner/scan_ops.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use super::analysis::{PlanningContext, RelationshipInstance};
99
use crate::ast::PropertyValue;
1010
use crate::case_insensitive::qualify_column;
1111
use crate::error::Result;
12-
use crate::source_catalog::GraphSourceCatalog;
1312
use datafusion::logical_expr::{col, BinaryExpr, Expr, LogicalPlan, LogicalPlanBuilder, Operator};
13+
use lance_graph_catalog::GraphSourceCatalog;
1414
use std::collections::{HashMap, HashSet};
1515
use std::sync::Arc;
1616

@@ -107,7 +107,7 @@ impl DataFusionPlanner {
107107
// No catalog attached - create empty source fallback for flexibility
108108
// This allows planners created with DataFusionPlanner::new() to work
109109
// without requiring a catalog, though they won't have actual data sources
110-
let empty_source = Arc::new(crate::source_catalog::SimpleTableSource::empty());
110+
let empty_source = Arc::new(lance_graph_catalog::SimpleTableSource::empty());
111111
let normalized_label = label.to_lowercase();
112112
let builder =
113113
LogicalPlanBuilder::scan(&normalized_label, empty_source, None).map_err(|e| {

crates/lance-graph/src/datafusion_planner/test_fixtures.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
66
use crate::config::GraphConfig;
77
use crate::logical_plan::LogicalOperator;
8-
use crate::source_catalog::{InMemoryCatalog, SimpleTableSource};
98
use arrow_schema::{DataType, Field, Schema};
9+
use lance_graph_catalog::{InMemoryCatalog, SimpleTableSource};
1010
use std::sync::Arc;
1111

1212
pub fn person_schema() -> Arc<Schema> {
@@ -17,7 +17,7 @@ pub fn person_schema() -> Arc<Schema> {
1717
]))
1818
}
1919

20-
pub fn make_catalog() -> Arc<dyn crate::source_catalog::GraphSourceCatalog> {
20+
pub fn make_catalog() -> Arc<dyn lance_graph_catalog::GraphSourceCatalog> {
2121
let person_src = Arc::new(SimpleTableSource::new(person_schema()));
2222
let knows_schema = Arc::new(Schema::new(vec![
2323
Field::new("src_person_id", DataType::Int64, false),

crates/lance-graph/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,18 @@ pub mod error;
4343
pub mod lance_native_planner;
4444
pub mod lance_vector_search;
4545
pub mod logical_plan;
46-
pub mod namespace;
4746
pub mod parser;
4847
pub mod query;
4948
pub mod semantic;
5049
pub mod simple_executor;
51-
pub mod source_catalog;
5250

5351
/// Maximum allowed hops for variable-length relationship expansion (e.g., *1..N)
5452
pub const MAX_VARIABLE_LENGTH_HOPS: u32 = 20;
5553

5654
pub use config::{GraphConfig, NodeMapping, RelationshipMapping};
5755
pub use error::{GraphError, Result};
56+
pub use lance_graph_catalog::{
57+
DirNamespace, GraphSourceCatalog, InMemoryCatalog, SimpleTableSource,
58+
};
5859
pub use lance_vector_search::VectorSearch;
59-
pub use namespace::DirNamespace;
6060
pub use query::{CypherQuery, ExecutionStrategy};

crates/lance-graph/src/namespace/directory.rs

Lines changed: 0 additions & 3 deletions
This file was deleted.

crates/lance-graph/src/namespace/mod.rs

Lines changed: 0 additions & 3 deletions
This file was deleted.

crates/lance-graph/src/query.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ use crate::ast::ReadingClause;
88
use crate::config::GraphConfig;
99
use crate::error::{GraphError, Result};
1010
use crate::logical_plan::LogicalPlanner;
11-
use crate::namespace::DirNamespace;
1211
use crate::parser::parse_cypher_query;
1312
use crate::simple_executor::{
1413
to_df_boolean_expr_simple, to_df_order_by_expr_simple, to_df_value_expr_simple, PathExecutor,
1514
};
1615
use arrow_array::RecordBatch;
1716
use arrow_schema::{Field, Schema, SchemaRef};
17+
use lance_graph_catalog::DirNamespace;
1818
use lance_namespace::models::DescribeTableRequest;
1919
use std::collections::{HashMap, HashSet};
2020
use std::sync::Arc;
@@ -395,8 +395,8 @@ impl CypherQuery {
395395
&self,
396396
ctx: datafusion::execution::context::SessionContext,
397397
) -> Result<arrow::record_batch::RecordBatch> {
398-
use crate::source_catalog::InMemoryCatalog;
399398
use datafusion::datasource::DefaultTableSource;
399+
use lance_graph_catalog::InMemoryCatalog;
400400
use std::sync::Arc;
401401

402402
let config = self.require_config()?;
@@ -463,7 +463,7 @@ impl CypherQuery {
463463
/// ```ignore
464464
/// use std::sync::Arc;
465465
/// use datafusion::execution::context::SessionContext;
466-
/// use lance_graph::source_catalog::InMemoryCatalog;
466+
/// use lance_graph::InMemoryCatalog;
467467
/// use lance_graph::query::CypherQuery;
468468
///
469469
/// // Create custom catalog
@@ -481,7 +481,7 @@ impl CypherQuery {
481481
/// ```
482482
pub async fn execute_with_catalog_and_context(
483483
&self,
484-
catalog: std::sync::Arc<dyn crate::source_catalog::GraphSourceCatalog>,
484+
catalog: std::sync::Arc<dyn lance_graph_catalog::GraphSourceCatalog>,
485485
ctx: datafusion::execution::context::SessionContext,
486486
) -> Result<arrow::record_batch::RecordBatch> {
487487
use arrow::compute::concat_batches;
@@ -557,12 +557,12 @@ impl CypherQuery {
557557
&self,
558558
datasets: HashMap<String, arrow::record_batch::RecordBatch>,
559559
) -> Result<(
560-
crate::source_catalog::InMemoryCatalog,
560+
lance_graph_catalog::InMemoryCatalog,
561561
datafusion::execution::context::SessionContext,
562562
)> {
563-
use crate::source_catalog::InMemoryCatalog;
564563
use datafusion::datasource::{DefaultTableSource, MemTable};
565564
use datafusion::execution::context::SessionContext;
565+
use lance_graph_catalog::InMemoryCatalog;
566566
use std::sync::Arc;
567567

568568
if datasets.is_empty() {
@@ -619,13 +619,13 @@ impl CypherQuery {
619619
&self,
620620
namespace: std::sync::Arc<dyn lance_namespace::LanceNamespace + Send + Sync>,
621621
) -> Result<(
622-
crate::source_catalog::InMemoryCatalog,
622+
lance_graph_catalog::InMemoryCatalog,
623623
datafusion::execution::context::SessionContext,
624624
)> {
625-
use crate::source_catalog::InMemoryCatalog;
626625
use datafusion::datasource::{DefaultTableSource, TableProvider};
627626
use datafusion::execution::context::SessionContext;
628627
use lance::datafusion::LanceTableProvider;
628+
use lance_graph_catalog::InMemoryCatalog;
629629
use std::sync::Arc;
630630

631631
let config = self.require_config()?;
@@ -740,7 +740,7 @@ impl CypherQuery {
740740
/// Internal helper to explain the query execution plan with explicit catalog and session context
741741
async fn explain_internal(
742742
&self,
743-
catalog: std::sync::Arc<dyn crate::source_catalog::GraphSourceCatalog>,
743+
catalog: std::sync::Arc<dyn lance_graph_catalog::GraphSourceCatalog>,
744744
ctx: datafusion::execution::context::SessionContext,
745745
) -> Result<String> {
746746
// Create all plans (phases 1-4)
@@ -757,7 +757,7 @@ impl CypherQuery {
757757
/// DataFusion logical planning) without creating the physical plan.
758758
fn create_logical_plans(
759759
&self,
760-
catalog: std::sync::Arc<dyn crate::source_catalog::GraphSourceCatalog>,
760+
catalog: std::sync::Arc<dyn lance_graph_catalog::GraphSourceCatalog>,
761761
) -> Result<(
762762
crate::logical_plan::LogicalOperator,
763763
datafusion::logical_expr::LogicalPlan,
@@ -791,7 +791,7 @@ impl CypherQuery {
791791
/// Helper to create all plans (graph logical, DataFusion logical, physical)
792792
async fn create_plans(
793793
&self,
794-
catalog: std::sync::Arc<dyn crate::source_catalog::GraphSourceCatalog>,
794+
catalog: std::sync::Arc<dyn lance_graph_catalog::GraphSourceCatalog>,
795795
ctx: &datafusion::execution::context::SessionContext,
796796
) -> Result<(
797797
crate::logical_plan::LogicalOperator,

crates/lance-graph/src/source_catalog.rs

Lines changed: 0 additions & 4 deletions
This file was deleted.

0 commit comments

Comments
 (0)