From 92a1ab25f41f994736c60a28e79bfaa54aaf9b6f Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 4 Sep 2025 18:41:46 -0700 Subject: [PATCH] address comments --- SQL_FEATURE_PLAN.md | 7 ++++ weed/command/db.go | 2 +- weed/query/engine/describe.go | 43 +++++++-------------- weed/query/engine/hybrid_message_scanner.go | 3 +- 4 files changed, 22 insertions(+), 33 deletions(-) diff --git a/SQL_FEATURE_PLAN.md b/SQL_FEATURE_PLAN.md index 3296b5920..893328c69 100644 --- a/SQL_FEATURE_PLAN.md +++ b/SQL_FEATURE_PLAN.md @@ -41,6 +41,13 @@ To provide a SQL querying interface for SeaweedFS, enabling analytics on existin **1. Scaffolding & Dependencies** +* **PostgreSQL Driver:** `github.com/lib/pq v1.10.9` - Provides PostgreSQL wire protocol compatibility for client connections +* **PostgreSQL Connection Pool:** `github.com/jackc/pgx/v5 v5.7.5` - High-performance PostgreSQL driver with connection pooling and prepared statements +* **Parquet Processing:** `github.com/parquet-go/parquet-go v0.25.1` - Native Go Parquet reader/writer for columnar data processing +* **SQL Parser:** Custom PostgreSQL-compatible parser built without CGO dependencies for optimal performance +* **Query Engine Infrastructure:** New `weed/query/engine/` package providing comprehensive SQL execution framework +* **Schema Catalog:** Integration with existing `weed/mq/schema/` infrastructure for metadata management + **2. SQL Engine Architecture** diff --git a/weed/command/db.go b/weed/command/db.go index 3fa99103d..4173df9d8 100644 --- a/weed/command/db.go +++ b/weed/command/db.go @@ -37,7 +37,7 @@ func init() { dbOptions.port = cmdDB.Flag.Int("port", 5432, "Database server port") dbOptions.masterAddr = cmdDB.Flag.String("master", "localhost:9333", "SeaweedFS master server address") dbOptions.authMethod = cmdDB.Flag.String("auth", "trust", "Authentication method: trust, password, md5") - dbOptions.users = cmdDB.Flag.String("users", "", "User credentials for auth (format: user1:pass1;user2:pass2)") + dbOptions.users = cmdDB.Flag.String("users", "", "User credentials for auth (format: user1:pass1;user2:pass2). Note: passwords cannot contain semicolons") dbOptions.database = cmdDB.Flag.String("database", "default", "Default database name") dbOptions.maxConns = cmdDB.Flag.Int("max-connections", 100, "Maximum concurrent connections") dbOptions.idleTimeout = cmdDB.Flag.String("idle-timeout", "1h", "Connection idle timeout") diff --git a/weed/query/engine/describe.go b/weed/query/engine/describe.go index 19d71f3dd..489c4e8e8 100644 --- a/weed/query/engine/describe.go +++ b/weed/query/engine/describe.go @@ -94,24 +94,16 @@ func (e *SQLEngine) executeShowStatementWithDescribe(ctx context.Context, stmt * // Use schema field if set by parser database = stmt.Schema } else { - // Try to get from OnTable.Name safely with recovery - func() { - defer func() { - if r := recover(); r != nil { - // If we panic, just use current database - database = e.catalog.GetCurrentDatabase() - } - }() - if stmt.OnTable.Name != nil { - if nameStr := stmt.OnTable.Name.String(); nameStr != "" { - database = nameStr - } else { - database = e.catalog.GetCurrentDatabase() - } + // Try to get from OnTable.Name with proper nil checks + if stmt.OnTable.Name != nil { + if nameStr := stmt.OnTable.Name.String(); nameStr != "" { + database = nameStr } else { database = e.catalog.GetCurrentDatabase() } - }() + } else { + database = e.catalog.GetCurrentDatabase() + } } if database == "" { // Use current database context @@ -122,22 +114,13 @@ func (e *SQLEngine) executeShowStatementWithDescribe(ctx context.Context, stmt * // SHOW COLUMNS FROM table is equivalent to DESCRIBE var tableName, database string - // Safely extract table name and database - func() { - defer func() { - if r := recover(); r != nil { - // If we panic, use empty values which will cause fallthrough - tableName = "" - database = "" - } - }() - if stmt.OnTable.Name != nil { - tableName = stmt.OnTable.Name.String() - if stmt.OnTable.Qualifier != nil { - database = stmt.OnTable.Qualifier.String() - } + // Safely extract table name and database with proper nil checks + if stmt.OnTable.Name != nil { + tableName = stmt.OnTable.Name.String() + if stmt.OnTable.Qualifier != nil { + database = stmt.OnTable.Qualifier.String() } - }() + } if tableName != "" { return e.executeDescribeStatement(ctx, tableName, database) diff --git a/weed/query/engine/hybrid_message_scanner.go b/weed/query/engine/hybrid_message_scanner.go index 84dcf0730..a7fba21bf 100644 --- a/weed/query/engine/hybrid_message_scanner.go +++ b/weed/query/engine/hybrid_message_scanner.go @@ -202,8 +202,7 @@ func (hms *HybridMessageScanner) ScanWithStats(ctx context.Context, options Hybr // Get all partitions for this topic via MQ broker discovery partitions, err := hms.discoverTopicPartitions(ctx) if err != nil { - // Fallback to default partition if discovery fails - partitions = []topic.Partition{{RangeStart: 0, RangeStop: 1000}} + return nil, stats, fmt.Errorf("failed to discover partitions for topic %s: %v", hms.topic.String(), err) } stats.PartitionsScanned = len(partitions)