Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Named parameter support and update Thrift #155

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Named parameter support and update Thrift #155

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57723a3
Named parameter support and update Thrift
rcypher-databricks Aug 16, 2023
340333c
Added back in unsupported error on parameterized query
rcypher-databricks Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 80 additions & 6 deletions connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dbsql
import (
"context"
"database/sql/driver"
"fmt"
"time"

"github.com/databricks/databricks-sql-go/driverctx"
Expand Down Expand Up @@ -100,9 +101,6 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
defer log.Duration(msg, start)

ctx = driverctx.NewContextWithConnId(ctx, c.id)
if len(args) > 0 {
andrefurlan-db marked this conversation as resolved.
Show resolved Hide resolved
return nil, dbsqlerrint.NewDriverError(ctx, dbsqlerr.ErrParametersNotSupported, nil)
}
exStmtResp, opStatusResp, err := c.runQuery(ctx, query, args)

if exStmtResp != nil && exStmtResp.OperationHandle != nil {
Expand Down Expand Up @@ -142,9 +140,7 @@ func (c *conn) QueryContext(ctx context.Context, query string, args []driver.Nam
msg, start := log.Track("QueryContext")

ctx = driverctx.NewContextWithConnId(ctx, c.id)
if len(args) > 0 {
return nil, dbsqlerrint.NewDriverError(ctx, dbsqlerr.ErrParametersNotSupported, nil)
}

// first we try to get the results synchronously.
// at any point in time that the context is done we must cancel and return
exStmtResp, opStatusResp, err := c.runQuery(ctx, query, args)
Expand Down Expand Up @@ -284,6 +280,7 @@ func (c *conn) executeStatement(ctx context.Context, query string, args []driver
MaxRows: int64(c.cfg.MaxRows),
},
CanDecompressLZ4Result_: &c.cfg.UseLz4Compression,
Parameters: namedValuesToTSparkParams(args),
}

if c.cfg.UseArrowBatches {
Expand Down Expand Up @@ -337,6 +334,83 @@ func (c *conn) executeStatement(ctx context.Context, query string, args []driver
return resp, err
}

func namedValuesToTSparkParams(args []driver.NamedValue) []*cli_service.TSparkParameter {
var ts []string = []string{"STRING", "DOUBLE", "BOOLEAN"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all number values be "DOUBLE"? What about INT, or DECIMAL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment TSparkParameterValue only support bool, double and string values. I'm waiting for clarification on the question of losing precision when converted to double. Also waiting for clarification on whether TSparkParameter.Type should correspond to the type in TSparkParameterValue or if it is used for converting the value.

Copy link
Contributor

Choose a reason for hiding this comment

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

From the code I saw in DBR, TSparkParameter.Type should correspond to the data types listed here. In DBR, there will be an attempt to cast these to the correct type. Floats can even be represented as a string so we could do this (pseudocode):

TSparkParameter{Type: "FLOAT", Value: TSparkParameter{stringValue: "1234.125"}, ...}

Or

TSparkParameter{Type: "DECIMAL(6, 2)", Value: TSparkParameter{stringValue: "1234.12"}, ...}

Or

TSparkParameter{Type: "BOOLEAN", Value: TSparkParameter{stringValue: "false"}, ...}

I think "ARRAY", "MAP", "STRUCT", and "BINARY" are not supported here however

var params []*cli_service.TSparkParameter
for i := range args {
arg := args[i]
param := cli_service.TSparkParameter{Value: &cli_service.TSparkParameterValue{}}
if arg.Name != "" {
param.Name = &arg.Name
} else {
i := int32(arg.Ordinal)
param.Ordinal = &i
}

switch t := arg.Value.(type) {
case bool:
b := arg.Value.(bool)
param.Value.BooleanValue = &b
param.Type = &ts[2]
case string:
s := arg.Value.(string)
param.Value.StringValue = &s
param.Type = &ts[0]
case int:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case uint:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case int8:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case uint8:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case int16:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case uint16:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case int32:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case uint32:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case int64:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case uint64:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
case float32:
f := float64(t)
param.Value.DoubleValue = &f
param.Type = &ts[1]
default:
s := fmt.Sprintf("%s", arg.Value)
param.Value.StringValue = &s
param.Type = &ts[0]
}

params = append(params, &param)
}
return params
}

func (c *conn) pollOperation(ctx context.Context, opHandle *cli_service.TOperationHandle) (*cli_service.TGetOperationStatusResp, error) {
corrId := driverctx.CorrelationIdFromContext(ctx)
log := logger.WithContext(c.id, corrId, client.SprintGuid(opHandle.OperationId.GUID))
Expand Down
1 change: 0 additions & 1 deletion internal/cli_service/README.md
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion internal/cli_service/cli_service-consts.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading