Skip to content

Commit b548cb4

Browse files
committed
refactor: Address Copilot PR review feedback for production readiness
🔧 Improvements based on Copilot's code review: ✅ Removed debug print statements from production code - Removed fmt.Printf debug statements from migrator.go FullDataTypeOf method - Cleaned up production code to remove debug noise ✅ Implemented controlled debug logging system - Added GORM_DUCKDB_DEBUG environment variable control - Created debugLog() function that only logs when debug mode enabled - Added errorLog() function for consistent error logging - Replaced all log.Printf("[DEBUG]") with controlled debugLog() calls - Replaced all log.Printf("[ERROR]") with errorLog() calls ✅ Fixed error handling in HasTable method - Changed error masking (return nil) to proper error propagation (return err) - Added descriptive error message for nil rows condition - Improved debugging experience by preserving actual error information 🎯 Production Benefits: - Cleaner production logs (no debug noise unless explicitly enabled) - Better error debugging with proper error propagation - Configurable logging via GORM_DUCKDB_DEBUG environment variable - Maintained all functionality while improving code quality Usage: Set GORM_DUCKDB_DEBUG=true to enable detailed debug logging
1 parent 707d095 commit b548cb4

File tree

3 files changed

+93
-79
lines changed

3 files changed

+93
-79
lines changed

custom_row_debug.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,49 @@
11
package duckdb
22

33
import (
4-
"log"
5-
64
"gorm.io/gorm"
75
)
86

97
// CustomRowQuery is a debugging version of GORM's RowQuery callback
108
func CustomRowQuery(db *gorm.DB) {
11-
log.Printf("[DEBUG] CustomRowQuery called")
12-
log.Printf("[DEBUG] db.Error: %v", db.Error)
13-
log.Printf("[DEBUG] db.DryRun: %t", db.DryRun)
9+
debugLog(" CustomRowQuery called")
10+
debugLog(" db.Error: %v", db.Error)
11+
debugLog(" db.DryRun: %t", db.DryRun)
1412

1513
if db.Error == nil {
16-
log.Printf("[DEBUG] No error, calling BuildQuerySQL")
14+
debugLog(" No error, calling BuildQuerySQL")
1715
// This is what GORM's BuildQuerySQL does for Raw queries
1816
if db.Statement.SQL.Len() == 0 {
19-
log.Printf("[DEBUG] SQL is empty, this shouldn't happen for Raw() queries")
17+
debugLog(" SQL is empty, this shouldn't happen for Raw() queries")
2018
}
2119

2220
// Check for DryRun or Error before proceeding
2321
if db.DryRun || db.Error != nil {
24-
log.Printf("[DEBUG] DryRun=%t or Error=%v, returning early", db.DryRun, db.Error)
22+
debugLog(" DryRun=%t or Error=%v, returning early", db.DryRun, db.Error)
2523
return
2624
}
2725

28-
log.Printf("[DEBUG] Checking for 'rows' setting")
26+
debugLog(" Checking for 'rows' setting")
2927
if isRows, ok := db.Get("rows"); ok && isRows.(bool) {
30-
log.Printf("[DEBUG] isRows=true, calling QueryContext")
28+
debugLog(" isRows=true, calling QueryContext")
3129
db.Statement.Settings.Delete("rows")
3230
db.Statement.Dest, db.Error = db.Statement.ConnPool.QueryContext(db.Statement.Context, db.Statement.SQL.String(), db.Statement.Vars...)
3331
} else {
34-
log.Printf("[DEBUG] isRows=false or not found, calling QueryRowContext")
35-
log.Printf("[DEBUG] SQL: %s", db.Statement.SQL.String())
36-
log.Printf("[DEBUG] Vars: %v", db.Statement.Vars)
37-
log.Printf("[DEBUG] ConnPool type: %T", db.Statement.ConnPool)
32+
debugLog(" isRows=false or not found, calling QueryRowContext")
33+
debugLog(" SQL: %s", db.Statement.SQL.String())
34+
debugLog(" Vars: %v", db.Statement.Vars)
35+
debugLog(" ConnPool type: %T", db.Statement.ConnPool)
3836

3937
result := db.Statement.ConnPool.QueryRowContext(db.Statement.Context, db.Statement.SQL.String(), db.Statement.Vars...)
40-
log.Printf("[DEBUG] QueryRowContext returned: %v (nil: %t)", result, result == nil)
38+
debugLog(" QueryRowContext returned: %v (nil: %t)", result, result == nil)
4139

4240
db.Statement.Dest = result
43-
log.Printf("[DEBUG] After assignment - Statement.Dest: %v (nil: %t)", db.Statement.Dest, db.Statement.Dest == nil)
41+
debugLog(" After assignment - Statement.Dest: %v (nil: %t)", db.Statement.Dest, db.Statement.Dest == nil)
4442
}
4543

46-
log.Printf("[DEBUG] Setting RowsAffected to -1")
44+
debugLog(" Setting RowsAffected to -1")
4745
db.RowsAffected = -1
4846
} else {
49-
log.Printf("[DEBUG] db.Error is not nil: %v", db.Error)
47+
debugLog(" db.Error is not nil: %v", db.Error)
5048
}
5149
}

0 commit comments

Comments
 (0)