First aproach to new driver... reusing code... #21

josejuanmontiel · 2023-01-28T11:12:25Z

The idea... include new driver reusing actual code... for later dinamically load by configuration.

With the two (three) actual driver ... use ifs inside actual backend... because new one of new drivers change the code more drastically, decide to implements new backend.

And to reuse actual code in backend that don't change... in this backend include the "parent" of sql backend (the actual one) and only reimplement those methods that didn't change.

The trick its done here

type csvqDBWriter struct {
sqlDBWriter
}

reusing the actual implementation ... and only change the method ... that realy change...

josejuanmontiel · 2023-01-30T10:19:22Z

cmd/main.go

+	dbTypeOracle = backends.DbTypeOracle
+	dbTypeCsvq   = backends.DbTypeCsvq


New constants for the new drivers..

josejuanmontiel · 2023-01-30T10:19:40Z

cmd/main.go

+
+	// TODO - Adding drivers...
+	_ "github.com/mithrandie/csvq-driver" // sql.Open("csvq", "/path/to/data/directory")
+	_ "github.com/sijms/go-ora/v2"        // sql.Open("oracle", "oracle://OT:yourpassword@localhost/XE")


Here add new drivers... only imports...

josejuanmontiel · 2023-01-30T10:21:06Z

cmd/main.go

+		case dbTypeCsvq:
+			backend, errI = backends.NewCsvqBackend(conn, opt, sLog)


Because with this driver... the code change a few... decide implement new backend .. this aproach (not use ifs with typo) could help later to load this code dinamically

josejuanmontiel · 2023-01-30T10:24:01Z

backends/backends.go

@@ -22,4 +24,50 @@ type ResultSet interface {
 	WriteRow([]interface{}) error
 	Flush() error
 	Close() error
+	CreateTableSchema(cols []string, colTypes []*sql.ColumnType) insertSchema


This methods it a TODO may could be private as earlier...

josejuanmontiel · 2023-01-30T10:24:43Z

backends/backends.go

+type SqlDB struct {
+	db     *sql.DB
+	opt    Opt
+	logger *log.Logger
+
+	// The result schemas (CREATE TABLE ...) are dynamically
+	// generated everytime queries are executed based on their result columns.
+	// They're cached here so as to avoid repetetive generation.
+	resTableSchemas map[string]insertSchema
+	schemaMutex     sync.RWMutex
+}
+
+type Writer struct {
+	jobID       string
+	taskName    string
+	colsWritten bool
+	cols        []string
+	rows        [][]byte
+	tx          *sql.Tx
+	tbl         string
+}


Move generic struct to interface... and other related things...

josejuanmontiel · 2023-01-30T10:25:16Z

backends/sqldb.go

-	// They're cached here so as to avoid repetetive generation.
-	resTableSchemas map[string]insertSchema
-	schemaMutex     sync.RWMutex
+	SqlDB


Use the generic type extacted to interface...

josejuanmontiel · 2023-01-30T10:27:03Z

backends/sqldb.go

+	Writer
+	SqlDB


Similar than before .. but less verbose :)

josejuanmontiel · 2023-01-30T10:27:42Z

backends/sqldb.go

-		resTableSchemas: make(map[string]insertSchema),
-		schemaMutex:     sync.RWMutex{},
-		logger:          l,
+		SqlDB{


Use generic type...

josejuanmontiel · 2023-01-30T10:27:47Z

backends/sqldb.go

-		backend:  s,
-		tbl:      fmt.Sprintf(s.opt.ResultsTable, jobID),
-		tx:       tx,
+		Writer{


Use generic type...

josejuanmontiel · 2023-01-30T10:28:50Z

backends/sqldb.go

+	w.schemaMutex.RLock()
+	_, ok := w.resTableSchemas[w.taskName]
+	w.schemaMutex.RUnlock()


Now we dont nedd backend.. it's an aggreation..

josejuanmontiel · 2023-01-30T10:30:15Z

backends/csvq.go

+type csvqDBWriter struct {
+	sqlDBWriter
+}


Here it's the tricky part ... this new backend implements the actual... but overwrite the only needed functionality...

josejuanmontiel · 2023-01-30T10:30:39Z

backends/csvq.go

+		return nil, err
+	}
+
+	return &csvqDBWriter{


Return it's own type of backend...

josejuanmontiel · 2023-01-30T10:31:25Z

backends/csvq.go

+	ins := fmt.Sprintf(`INSERT INTO "%%s" (%s) `, strings.Join(colNameHolder, ","))
+	ins += fmt.Sprintf("VALUES (%s)", strings.Join(colValHolder, ","))


The insert command change in csvq driver... and better than ifs... (as comented in other place) use a new backend...

josejuanmontiel · 2023-01-30T10:31:52Z

backends/csvq.go

+	// TODO - This should configurable... maybe in some case not supported or need incremental update..
+	// if _, err := tx.Exec(fmt.Sprintf(rSchema.dropTable, w.tbl)); err != nil {
+	// 	return err
+	// }


This driver don't need delete the table...

josejuanmontiel · 2023-01-30T10:32:12Z

backends/csvq.go

+	return insertSchema{
+		// TODO - In some dialects...
+		dropTable:   `DROP TABLE IF EXISTS "%s";`,
+		createTable: fmt.Sprintf(`CREATE %s TABLE %%s (%s);`, unlogged, strings.Join(fields, ",")),
+		insertRow:   fmt.Sprintf(`INSERT INTO %%s (%s) VALUES (%s)`, strings.Join(colNameHolder, ","), strings.Join(colValHolder, ",")),


And the sql change a little...

knadh · 2023-01-31T05:08:53Z

Hi @josejuanmontiel. Thanks for the inline comments on the PR, but could you please explain here in a few bullet points, what exactly is changing, why, and how it helps. The overall architecture.

josejuanmontiel · 2023-02-09T22:37:00Z

Hi!

What exactly is changing? There is an interface for the backends (before now, only one) the change consider this backend as parent backend to others, that only need to override the methods they change for new functionaly of this "driver"...
why? Splitting the new functionality to connect new database in different backend, organize the code better to future loading dinamically.
and how it helps? It could lead to remove the "if postgres" in some places ... if split the sql backend in two .. for mysql and postgres.
The overall architecture. To reuse the code in other backend need a few minor changes

CreateTableSchema goes public to be called from jobber.go before w.RegisterColTypes(cols, colTypes)
Move to the interfaces some types to organize
Now types sqlDB and sqlDBWriter aggrete the fields, that leads to remove the name attribute backend *sqlDB from sqlDBWriter
CreateTableSchema have the lock inside (not ouside)
The new backend (csv) only implements the methods that have "sql code" that change in this database.

As bonus, need some extra configuration too... i include the comented option to run the jobber_test.go with testcontainer.

TODO:

Adding some free CI ¿circleci?
"Parametrizing" the it test with test container (adding // +build integration)
Adding some test for the new csvq driver

knadh · 2023-02-10T05:52:57Z

Thanks for the update. Will try to review this some time soon.

knadh · 2023-09-22T06:28:19Z

Please see this #23 (comment)

WIP First aproach to new driver... reusing code...

aa7083a

josejuanmontiel mentioned this pull request Jan 28, 2023

Adding new drivers #18

Closed

josejuanmontiel commented Jan 30, 2023

View reviewed changes

backends/sqldb.go

Comment on lines +21 to +22

Writer

SqlDB

Copy link

Author

josejuanmontiel Jan 30, 2023

Choose a reason for hiding this comment

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

Similar than before .. but less verbose :)

josejuanmontiel commented Jan 30, 2023

View reviewed changes

backends/sqldb.go

backend: s,

tbl: fmt.Sprintf(s.opt.ResultsTable, jobID),

tx: tx,

Writer{

Copy link

Author

josejuanmontiel Jan 30, 2023

Choose a reason for hiding this comment

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

Use generic type...

josejuanmontiel commented Jan 30, 2023

View reviewed changes

backends/csvq.go

return nil, err

}

return &csvqDBWriter{

Copy link

Author

josejuanmontiel Jan 30, 2023

Choose a reason for hiding this comment

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

Return it's own type of backend...

josejuanmontiel commented Jan 30, 2023

View reviewed changes

josejuanmontiel added 2 commits February 6, 2023 22:59

Next iteration of refactor

870561e

WIP docker sample

7f727cc

josejuanmontiel marked this pull request as ready for review February 9, 2023 22:37

josejuanmontiel changed the title ~~WIP First aproach to new driver... reusing code...~~ First aproach to new driver... reusing code... Feb 9, 2023

josejuanmontiel mentioned this pull request Feb 9, 2023

Sample of bulk insert #23

Closed

knadh closed this Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First aproach to new driver... reusing code... #21

First aproach to new driver... reusing code... #21

josejuanmontiel commented Jan 28, 2023 •

edited

Loading

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

josejuanmontiel Jan 30, 2023

knadh commented Jan 31, 2023

josejuanmontiel commented Feb 9, 2023

knadh commented Feb 10, 2023

knadh commented Sep 22, 2023

		dbTypeOracle = backends.DbTypeOracle
		dbTypeCsvq = backends.DbTypeCsvq

		case dbTypeCsvq:
		backend, errI = backends.NewCsvqBackend(conn, opt, sLog)

		ins := fmt.Sprintf(`INSERT INTO "%%s" (%s) `, strings.Join(colNameHolder, ","))
		ins += fmt.Sprintf("VALUES (%s)", strings.Join(colValHolder, ","))

First aproach to new driver... reusing code... #21

First aproach to new driver... reusing code... #21

Conversation

josejuanmontiel commented Jan 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knadh commented Jan 31, 2023

josejuanmontiel commented Feb 9, 2023

knadh commented Feb 10, 2023

knadh commented Sep 22, 2023

josejuanmontiel commented Jan 28, 2023 •

edited

Loading