Removed old comments and transferred todos to jira (#82)

Stale comments from development work removed. TODOs and call-outs
transferred to Jira (Go Driver v1.1 epic)

Signed-off-by: Matthew Kim
<[email protected]>

Author: mattdeekay

connection.go

@@ -117,9 +117,6 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
 		}
 	}
 	if err != nil {
-		// TODO: are there error situations in which the operation still needs to be closed?
-		// Currently if there is an error we never get back a TExecuteStatementResponse so
-		// can't try to close.
 		log.Err(err).Msgf("databricks: failed to execute query: query %s", query)
 		return nil, wrapErrf(err, "failed to execute query")
 	}
@@ -153,7 +150,6 @@ func (c *conn) QueryContext(ctx context.Context, query string, args []driver.Nam
 	defer log.Duration(msg, start)
 
 	if err != nil {
-		// gotta also think about close operation here
 		log.Err(err).Msgf("databricks: failed to run query: query %s", query)
 		return nil, wrapErrf(err, "failed to run query")
 	}
@@ -175,7 +171,6 @@ func (c *conn) runQuery(ctx context.Context, query string, args []driver.NamedVa
 	if err != nil {
 		return exStmtResp, nil, err
 	}
-	// hold on to the operation handle
 	opHandle := exStmtResp.OperationHandle
 	if opHandle != nil && opHandle.OperationId != nil {
 		log = logger.WithContext(
@@ -191,14 +186,12 @@ func (c *conn) runQuery(ctx context.Context, query string, args []driver.NamedVa
 		// terminal states
 		// good
 		case cli_service.TOperationState_FINISHED_STATE:
-			// return results
 			return exStmtResp, opStatus, nil
 		// bad
 		case cli_service.TOperationState_CANCELED_STATE,
 			cli_service.TOperationState_CLOSED_STATE,
 			cli_service.TOperationState_ERROR_STATE,
 			cli_service.TOperationState_TIMEDOUT_STATE:
-			// do we need to close the operation in these cases?
 			logBadQueryState(log, opStatus)
 			return exStmtResp, opStatus, errors.New(opStatus.GetDisplayMessage())
 		// live states
@@ -213,7 +206,6 @@ func (c *conn) runQuery(ctx context.Context, query string, args []driver.NamedVa
 			// terminal states
 			// good
 			case cli_service.TOperationState_FINISHED_STATE:
-				// return handle to fetch results later
 				return exStmtResp, statusResp, nil
 			// bad
 			case cli_service.TOperationState_CANCELED_STATE,
@@ -242,7 +234,6 @@ func (c *conn) runQuery(ctx context.Context, query string, args []driver.NamedVa
 		// terminal states
 		// good
 		case cli_service.TOperationState_FINISHED_STATE:
-			// return handle to fetch results later
 			return exStmtResp, statusResp, nil
 		// bad
 		case cli_service.TOperationState_CANCELED_STATE,
@@ -273,13 +264,9 @@ func (c *conn) executeStatement(ctx context.Context, query string, args []driver
 		Statement:     query,
 		RunAsync:      c.cfg.RunAsync,
 		QueryTimeout:  int64(c.cfg.QueryTimeout / time.Second),
-		// this is specific for databricks. It shortcuts server round trips
 		GetDirectResults: &cli_service.TSparkGetDirectResults{
 			MaxRows: int64(c.cfg.MaxRows),
 		},
-		// CanReadArrowResult_: &t,
-		// CanDecompressLZ4Result_: &f,
-		// CanDownloadResult_: &t,
 	}
 
 	ctx = driverctx.NewContextWithConnId(ctx, c.id)
@@ -339,7 +326,6 @@ func (c *conn) pollOperation(ctx context.Context, opHandle *cli_service.TOperati
 				log.Debug().Msgf("databricks: status %s", statusResp.GetOperationState().String())
 			}
 			return func() bool {
-				// which other states?
 				if err != nil {
 					return true
 				}

connector.go

@@ -34,7 +34,6 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
 		return nil, wrapErr(err, "error initializing thrift client")
 	}
 
-	// we need to ensure that open session will eventually end
 	session, err := tclient.OpenSession(ctx, &cli_service.TOpenSessionReq{
 		ClientProtocol: c.cfg.ThriftProtocolVersion,
 		Configuration:  make(map[string]string),

driver.go

@@ -45,41 +45,3 @@ func (d *databricksDriver) OpenConnector(dsn string) (driver.Connector, error) {
 
 var _ driver.Driver = (*databricksDriver)(nil)
 var _ driver.DriverContext = (*databricksDriver)(nil)
-
-// type databricksDB struct {
-// 	*sql.DB
-// }
-
-// func OpenDB(c driver.Connector) *databricksDB {
-// 	db := sql.OpenDB(c)
-// 	return &databricksDB{db}
-// }
-
-// func (db *databricksDB) QueryContextAsync(ctx context.Context, query string, args ...any) (rows *sql.Rows, queryId string, err error) {
-// 	return nil, "", nil
-// }
-
-// func (db *databricksDB) ExecContextAsync(ctx context.Context, query string, args ...any) (result sql.Result, queryId string) {
-// 	//go do something
-// 	return nil, ""
-// }
-
-// func (db *databricksDB) CancelQuery(ctx context.Context, queryId string) error {
-// 	//go do something
-// 	return nil
-// }
-
-// func (db *databricksDB) GetQueryStatus(ctx context.Context, queryId string) error {
-// 	//go do something
-// 	return nil
-// }
-
-// func (db *databricksDB) FetchRows(ctx context.Context, queryId string) (rows *sql.Rows, err error) {
-// 	//go do something
-// 	return nil, nil
-// }
-
-// func (db *databricksDB) FetchResult(ctx context.Context, queryId string) (rows sql.Result, err error) {
-// 	//go do something
-// 	return nil, nil
-// }

internal/client/client.go

@@ -167,12 +167,6 @@ func (tsc *ThriftServiceClient) CancelOperation(ctx context.Context, req *cli_se
 	return resp, CheckStatus(resp)
 }
 
-// log.Debug().Msg(fmt.Sprint(c.transport.response.StatusCode))
-// log.Debug().Msg(c.transport.response.Header.Get("X-Databricks-Org-Id"))
-// log.Debug().Msg(c.transport.response.Header.Get("x-databricks-error-or-redirect-message"))
-// log.Debug().Msg(c.transport.response.Header.Get("x-thriftserver-error-message"))
-// log.Debug().Msg(c.transport.response.Header.Get("x-databricks-reason-phrase"))
-
 // InitThriftClient is a wrapper of the http transport, so we can have access to response code and headers.
 // It is important to know the code and headers to know if we need to retry or not
 func InitThriftClient(cfg *config.Config) (*ThriftServiceClient, error) {

internal/config/config.go

@@ -147,7 +147,7 @@ func WithDefaults() *Config {
 		ClientTimeout:             900 * time.Second,
 		PingTimeout:               15 * time.Second,
 		CanUseMultipleCatalogs:    true,
-		DriverName:                "godatabrickssqlconnector", //important. Do not change
+		DriverName:                "godatabrickssqlconnector", // important. Do not change
 		DriverVersion:             "0.9.0",
 		ThriftProtocol:            "binary",
 		ThriftTransport:           "http",

rows.go

@@ -147,7 +147,7 @@ func (r *rows) Next(dest []driver.Value) error {
 		return err
 	}
 
-	// populate the destinatino slice
+	// populate the destination slice
 	for i := range dest {
 		val, err := value(r.fetchResults.Results.Columns[i], metadata.Schema.Columns[i], r.nextRowIndex, r.location)
 
@@ -202,10 +202,9 @@ func (r *rows) ColumnTypeDatabaseTypeName(index int) string {
 }
 
 // ColumnTypeNullable returns a flag indicating whether the column is nullable
-// and an ok value of true if the status of the column is known.  Otherwise
+// and an ok value of true if the status of the column is known. Otherwise
 // a value of false is returned for ok.
 func (r *rows) ColumnTypeNullable(index int) (nullable, ok bool) {
-	// TODO: Update if we can figure out this information
 	return false, false
 }
 
@@ -216,8 +215,6 @@ func (r *rows) ColumnTypeLength(index int) (length int64, ok bool) {
 	}
 
 	typeName := getDBTypeID(columnInfo)
-	// TODO: figure out how to get better metadata about complex types
-	// currently map, array, and struct are returned as strings
 	switch typeName {
 	case cli_service.TTypeId_STRING_TYPE,
 		cli_service.TTypeId_VARCHAR_TYPE,
@@ -248,7 +245,6 @@ var (
 
 func getScanType(column *cli_service.TColumnDesc) reflect.Type {
 
-	// TODO: handle non-primitive types
 	entry := column.TypeDesc.Types[0].PrimitiveEntry
 
 	switch entry.Type {
@@ -289,15 +285,13 @@ func getScanType(column *cli_service.TColumnDesc) reflect.Type {
 }
 
 func getDBTypeName(column *cli_service.TColumnDesc) string {
-	// TODO: handle non-primitive types
 	entry := column.TypeDesc.Types[0].PrimitiveEntry
 	dbtype := strings.TrimSuffix(entry.Type.String(), "_TYPE")
 
 	return dbtype
 }
 
 func getDBTypeID(column *cli_service.TColumnDesc) cli_service.TTypeId {
-	// TODO: handle non-primitive types
 	entry := column.TypeDesc.Types[0].PrimitiveEntry
 	return entry.Type
 }
@@ -336,7 +330,6 @@ func (r *rows) getColumnMetadataByIndex(index int) (*cli_service.TColumnDesc, er
 		return nil, errors.Errorf("invalid column index: %d", index)
 	}
 
-	// tColumns := resultMetadata.Schema.GetColumns()
 	return columns[index], nil
 }
 
@@ -394,7 +387,7 @@ func (r *rows) fetchResultPage() error {
 
 	for !r.isNextRowInPage() {
 
-		// determine the direction of page fetching.  Currently we only handle
+		// determine the direction of page fetching. Currently we only handle
 		// TFetchOrientation_FETCH_PRIOR and TFetchOrientation_FETCH_NEXT
 		var direction cli_service.TFetchOrientation = r.getPageFetchDirection()
 		if direction == cli_service.TFetchOrientation_FETCH_PRIOR {

testserver.go

@@ -20,7 +20,6 @@ func (h *thriftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 func initThriftTestServer(handler cli_service.TCLIService) *httptest.Server {
 
-	// endpoint := fmt.Sprintf("%s:%d", cfg.Host, cfg.Port)
 	tcfg := &thrift.TConfiguration{
 		TLSConfig: nil,
 	}