Db read

[jason-costello]

See issue: #34
Allow reading from database:

• FromSQL, FromSQLContext, FromSQLTx, FromSQLTxContext functions
• Configurable NULL handling: "nil", "zero", "skip_row", custom map
• Type mapping: SQL types → Go types (int64, float64, string, bool, time.Time)
• Parameterized queries with args
• Transaction support
• Context support

[cs168898]

Do add drivers for postgres and mysql as well

[jason-costello]

fully implemented now for mysql, Postgres, and SQLite

[cs168898]

Just clarifying some stuff to make sure that it is intended, such as the ParseData field not being used and the missing imports and the import paths for the png files.

[cs168898]

The ParseDates field is not used. Since databases like SQLite often dont have a dedicated DATETIME type, it usually stores dates as strings 'TEXT' or numbers. Not sure if your intention is to store the table with a DataFrame full of date-strings?

[jason-costello]

addressed.

[cs168898]

Take note that the CI might not pass due to the 'test files' directory not being present in the repo.

[jason-costello]

I missed some items it appears when decoupling the read/write functions to their own branches. Now corrected.

[kishyassin]

The architecture is solid and the feature set is impressive. I've identified a few areas that need attention before merging

Must Fix Before Merge

1. (sql_dialect.go)

The CreateTableSQL method has a subtle but important bug:

// Line 72-75 - ISSUE: Map iteration order is random in Go
var columnDefs []string
for colName, colType := range columns {
    columnDefs = append(columnDefs, ...)
}

SQL has columns in random order, which could cause issues downstream. u can sort column names before iteration:

keys := make([]string, 0, len(columns))
for k := range columns {
    keys = append(keys, k)
}
sort.Strings(keys)
for _, colName := range keys {
    columnDefs = append(columnDefs, ...)
}

2. Binary Test Files in Repository

• goframe_tests/test_files/bar_plot_test.png
• goframe_tests/test_files/line_plot_test.png
  Remove these files or please fix this issue with

func TestVisualization(t *testing.T) {
    tmpDir := t.TempDir()  // Cleaned up automatically after test
    linePlotFilename := filepath.Join(tmpDir, "line_plot_test.png")
    // ... 
}

Then update test file paths:

- linePlotFilename := "test_files/line_plot_test.png"
+ linePlotFilename := filepath.Join(tmpDir, "line_plot_test.png")

3. ParseDates Semantics Unclear (sql_read.go)

I didnt understand ParseDates field in SQLReadOption from the first time 😂

// Line 23-24
ParseDates []string  // What exactly does this do?

please document it

4. Inconsistent NULL Handling (sql_read.go line 256)

The "zero" handler for NullTime returns nil instead of time.Time zero value:

case "zero":
    switch dest.(type) {
    // ... other types
    case *sql.NullTime:
        return nil, nil  // ← INCONSISTENT - should be time.Time{} or document why
    }

Either normalize all zero handlers or document the reason in comments.

---

5. Missing Input Validation

• No validation for empty table/column names
• No checks for NULL values in required parameters

Add basic validation:

func FromSQLContext(ctx context.Context, db *sql.DB, query string, args []any, ...) error {
    if db == nil {
        return fmt.Errorf("db cannot be nil")
    }
    if query == "" {
        return fmt.Errorf("query cannot be empty")
    }
    // ... etc etc
}

Great work! 🎉

[cs168898]

Please also remove the .png image files and maybe add a .gitignore for the test_files directory as image files will cause repo bloat, over time making git clone really really slow.

[kishyassin]

@cs168898 @jason-costello
I'm going to clode this PR as ignored
or merg it then request update