fix: redshift-data driver inconsistencies (#45)

# Description - Add missing mapping for `SharedConfigProfile` config option - Sanitize `RedshiftConfig` when passed to a `NewRedshiftConnector` to handle either a `ClusterIdentifier` config or a `WorkgroupName` config. - Handle fractional seconds for data types returning time. ## Security - [x] The code changed/added as part of this pull request won't create any security issues with how the software is being used.
rudderlabs · Apr 16, 2024 · 9df88a6 · 9df88a6
1 parent 15e0220
commit 9df88a6
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 35 deletions.
diff --git a/sqlconnect/internal/redshift/db.go b/sqlconnect/internal/redshift/db.go
@@ -92,19 +92,20 @@ func newRedshiftDataDB(credentialsJSON json.RawMessage) (*sql.DB, error) {
 		return nil, err
 	}
 	cfg := redshiftdriver.RedshiftConfig{
-		ClusterIdentifier: config.ClusterIdentifier,
-		Database:          config.Database,
-		DbUser:            config.User,
-		WorkgroupName:     config.WorkgroupName,
-		SecretsARN:        config.SecretsARN,
-		Region:            config.Region,
-		AccessKeyID:       config.AccessKeyID,
-		SecretAccessKey:   config.SecretAccessKey,
-		SessionToken:      config.SessionToken,
-		Timeout:           config.Timeout,
-		MinPolling:        config.MinPolling,
-		MaxPolling:        config.MaxPolling,
-		RetryMaxAttempts:  config.RetryMaxAttempts,
+		ClusterIdentifier:   config.ClusterIdentifier,
+		Database:            config.Database,
+		DbUser:              config.User,
+		WorkgroupName:       config.WorkgroupName,
+		SecretsARN:          config.SecretsARN,
+		Region:              config.Region,
+		AccessKeyID:         config.AccessKeyID,
+		SharedConfigProfile: config.SharedConfigProfile,
+		SecretAccessKey:     config.SecretAccessKey,
+		SessionToken:        config.SessionToken,
+		Timeout:             config.Timeout,
+		MinPolling:          config.MinPolling,
+		MaxPolling:          config.MaxPolling,
+		RetryMaxAttempts:    config.RetryMaxAttempts,
 	}
 	connector := redshiftdriver.NewRedshiftConnector(cfg)
 

diff --git a/sqlconnect/internal/redshift/driver/connector.go b/sqlconnect/internal/redshift/driver/connector.go
@@ -6,6 +6,7 @@ import (
 )
 
 func NewRedshiftConnector(cfg RedshiftConfig) driver.Connector {
+	cfg.Sanitize()
 	return &redshiftDataConnector{
 		d:   &redshiftDataDriver{},
 		cfg: &cfg,

diff --git a/sqlconnect/internal/redshift/driver/dsn.go b/sqlconnect/internal/redshift/driver/dsn.go
@@ -32,6 +32,15 @@ type RedshiftConfig struct {
 	Params url.Values
 }
 
+func (cfg *RedshiftConfig) Sanitize() {
+	if cfg.ClusterIdentifier != "" {
+		cfg.WorkgroupName = ""
+	}
+	if cfg.WorkgroupName != "" {
+		cfg.DbUser = ""
+	}
+}
+
 func (cfg *RedshiftConfig) LoadOpts() []func(*config.LoadOptions) error {
 	var opts []func(*config.LoadOptions) error
 	if cfg.SharedConfigProfile != "" {

diff --git a/sqlconnect/internal/redshift/driver/dsn_test.go b/sqlconnect/internal/redshift/driver/dsn_test.go
@@ -116,3 +116,29 @@ func TestRedshiftDataConfig__String(t *testing.T) {
 		})
 	}
 }
+
+func TestRedshiftConfigSanitize(t *testing.T) {
+	t.Run("cluster config", func(t *testing.T) {
+		c := RedshiftConfig{
+			ClusterIdentifier: "default",
+			WorkgroupName:     "default",
+			DbUser:            "admin",
+		}
+
+		c.Sanitize()
+		require.NotEmpty(t, c.ClusterIdentifier)
+		require.Empty(t, c.WorkgroupName)
+		require.NotEmpty(t, c.DbUser)
+	})
+
+	t.Run("workgroup config", func(t *testing.T) {
+		c := RedshiftConfig{
+			WorkgroupName: "default",
+			DbUser:        "admin",
+		}
+
+		c.Sanitize()
+		require.NotEmpty(t, c.WorkgroupName)
+		require.Empty(t, c.DbUser)
+	})
+}
diff --git a/sqlconnect/internal/redshift/driver/rows.go b/sqlconnect/internal/redshift/driver/rows.go
@@ -84,39 +84,39 @@ func (rows *redshiftRows) Next(dest []driver.Value) error {
 				dataType := strings.ToLower(*rows.page.ColumnMetadata[i].TypeName)
 				switch dataType {
 				case "timestamp":
-					t, err := time.Parse("2006-01-02 15:04:05", field.Value)
+					t, err := time.Parse("2006-01-02 15:04:05.999999", field.Value)
 					if err != nil {
 						dest[i] = field.Value
 					} else {
-						dest[i] = t.UTC().Format(time.RFC3339)
+						dest[i] = t.UTC().Format(time.RFC3339Nano)
 					}
 				case "timestamptz":
-					t, err := time.Parse("2006-01-02 15:04:05-07", field.Value)
+					t, err := time.Parse("2006-01-02 15:04:05.999999-07", field.Value)
 					if err != nil {
 						dest[i] = field.Value
 					} else {
-						dest[i] = t.UTC().Format(time.RFC3339)
+						dest[i] = t.UTC().Format(time.RFC3339Nano)
 					}
 				case "date":
 					t, err := time.Parse("2006-01-02", field.Value)
 					if err != nil {
 						dest[i] = field.Value
 					} else {
-						dest[i] = t.UTC().Format(time.RFC3339)
+						dest[i] = t.UTC().Format(time.RFC3339Nano)
 					}
 				case "time", "time without time zone":
-					t, err := time.Parse("15:04:05", field.Value)
+					t, err := time.Parse("15:04:05.999999", field.Value)
 					if err != nil {
 						dest[i] = field.Value
 					} else {
-						dest[i] = t.UTC().Format(time.RFC3339)
+						dest[i] = t.UTC().Format(time.RFC3339Nano)
 					}
 				case "timetz", "time with time zone":
-					t, err := time.Parse("15:04:05-07", field.Value)
+					t, err := time.Parse("15:04:05.999999-07", field.Value)
 					if err != nil {
 						dest[i] = field.Value
 					} else {
-						dest[i] = t.UTC().Format(time.RFC3339)
+						dest[i] = t.UTC().Format(time.RFC3339Nano)
 					}
 
 				default:

diff --git a/sqlconnect/internal/redshift/testdata/column-mapping-test-rows.json b/sqlconnect/internal/redshift/testdata/column-mapping-test-rows.json
@@ -21,12 +21,12 @@
         "_bpchar": "abc                                                                                                                                                                                                                                                             ",
         "_character": "abc       ",
         "_date": "2004-10-19T00:00:00Z",
-        "_time": "0000-01-01T10:23:54Z",
-        "_timetz": "0000-01-01T08:23:54Z",
-        "_timestamptz": "2004-10-19T08:23:54Z",
-        "_timestampntz": "2004-10-19T10:23:54Z",
-        "_timestampwtz": "2004-10-19T08:23:54Z",
-        "_timestamp": "2004-10-19T10:23:54Z",
+        "_time": "0000-01-01T10:23:54.123Z",
+        "_timetz": "0000-01-01T08:23:54.1234Z",
+        "_timestamptz": "2004-10-19T08:23:54.12345Z",
+        "_timestampntz": "2004-10-19T10:23:54.123456Z",
+        "_timestampwtz": "2004-10-19T08:23:54.123457Z",
+        "_timestamp": "2004-10-19T10:23:54.123457Z",
         "_boolean": true,
         "_bool": true
     },

diff --git a/sqlconnect/internal/redshift/testdata/column-mapping-test-seed.sql b/sqlconnect/internal/redshift/testdata/column-mapping-test-seed.sql
@@ -33,6 +33,6 @@ CREATE TABLE "{{.schema}}"."column_mappings_test" (
 INSERT INTO "{{.schema}}"."column_mappings_test"
     (_order, _int, _int2, _int4, _int8, _integer, _smallint, _bigint, _real, _float, _float4, _float8, _numeric, _double, _text, _varchar, _charvar, _nchar, _bpchar, _character, _date, _time, _timetz, _timestamptz, _timestampntz, _timestampwtz, _timestamp, _boolean, _bool)
 VALUES
-    (1, 1,    1,    1,    1,    1,    1,    1,    1,  1.1,  1,  1.1,  1.1,  1.1,  'abc', 'abc', 'abc', 'abc', 'abc',     'abc', '2004-10-19', '10:23:54', '10:23:54+02', '2004-10-19 10:23:54+02', '2004-10-19 10:23:54', '2004-10-19 10:23:54+02', '2004-10-19 10:23:54+02', true,  true ),
+    (1, 1,    1,    1,    1,    1,    1,    1,    1,  1.1,  1,  1.1,  1.1,  1.1,  'abc', 'abc', 'abc', 'abc', 'abc',     'abc', '2004-10-19', '10:23:54.123', '10:23:54.1234+02', '2004-10-19 10:23:54.12345+02', '2004-10-19 10:23:54.123456', '2004-10-19 10:23:54.1234567+02', '2004-10-19 10:23:54.12345678+02', true,  true ),
     (2, 0,    0,    0,    0,    0,    0,    0,    0,    0,    0,    0,    0,    0,    '',    '',    '',    '',    '',    '',    '2004-10-19', '10:23:54', '10:23:54+02', '2004-10-19 10:23:54+02', '2004-10-19 10:23:54', '2004-10-19 10:23:54+02', '2004-10-19 10:23:54+02', false, false), 
     (3, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,  NULL,  NULL,  NULL,  NULL,  NULL,  NULL,         NULL,       NULL,          NULL,                     NULL,                  NULL,                     NULL,                     NULL,  NULL );
diff --git a/sqlconnect/internal/redshift/testdata/legacy-column-mapping-test-rows.json b/sqlconnect/internal/redshift/testdata/legacy-column-mapping-test-rows.json
@@ -21,12 +21,12 @@
         "_bpchar": "abc                                                                                                                                                                                                                                                             ",
         "_character": "abc       ",
         "_date": "2004-10-19T00:00:00Z",
-        "_time": "0000-01-01T10:23:54Z",
-        "_timetz": "0000-01-01T08:23:54Z",
-        "_timestamptz": "2004-10-19T08:23:54Z",
-        "_timestampntz": "2004-10-19T10:23:54Z",
-        "_timestampwtz": "2004-10-19T08:23:54Z",
-        "_timestamp": "2004-10-19T10:23:54Z",
+        "_time": "0000-01-01T10:23:54.123Z",
+        "_timetz": "0000-01-01T08:23:54.1234Z",
+        "_timestamptz": "2004-10-19T08:23:54.12345Z",
+        "_timestampntz": "2004-10-19T10:23:54.123456Z",
+        "_timestampwtz": "2004-10-19T08:23:54.123457Z",
+        "_timestamp": "2004-10-19T10:23:54.123457Z",
         "_boolean": true,
         "_bool": true
     },