Skip to content

Commit

Permalink
Fix: V1 Frames have DateTypes not recognized
Browse files Browse the repository at this point in the history
V1 frames, while containing a ColumnType, often do not fill them in.  The code muust rely on DateType, which details what the C# type would be.

This issue exposed the following problems:

- Did not include the correct type for a bool, which was "boolean".  Possibly other types.
- Did not notice because Mgmt() was not used for anything but sending commands in our integraiton tests and code base

Fixes:
- Updated our internal integration test for the regression
- Added a new translation map that includes all values and aliases that Kusto docs says is supported.
- Made a slight tweak to a test, but did not add a new non-integration test, it would only be testing the existing table which isn't useful.
  • Loading branch information
John Doak authored and John Doak committed Mar 16, 2020
1 parent a17cdfa commit 3d32447
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 14 deletions.
7 changes: 4 additions & 3 deletions kusto/data/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ func W(inner error, outer error) error {
}

// OneToErr translates what we think is a Kusto OneApiError into an Error. If we don't recognize it, we return nil.
// This tries to wrap the internal errors, but because the errors we see don't conform to the current OneAPIError spec,
// I'm not sure what is going on. We shouldn't get a list of errors, but we do. We should get embedded. So I'm taking the guess
// that these are supposed to be wrapped errors.
// This tries to wrap the internal errors, but the errors that are generated are some type of early draft of OneApiError,
// not the current spec. Because the errors we see don't conform to the current OneAPIError spec, had to take guesses on
// what we will receive. The spec says we shouldn't get a list of errors, but we do(we should get an embedded error).
// So I'm taking the guess that these are supposed to be wrapped errors.
func OneToErr(m map[string]interface{}, op Op) *Error {
if m == nil {
return nil
Expand Down
4 changes: 2 additions & 2 deletions kusto/internal/frames/v1/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/Azure/azure-kusto-go/kusto/internal/frames"
)

// Decoder impolements frames.Decoder on the REST v1 frames.
// Decoder implements frames.Decoder on the REST v1 frames.
type Decoder struct {
dec *json.Decoder
op errors.Op
Expand Down Expand Up @@ -87,7 +87,7 @@ func (d *Decoder) findStringToken(s string) error {
}
}
}
return fmt.Errorf("(v1)found the end of the delimeter without finding token %q", s)
panic("unreachable")
}

func (d *Decoder) processTables(ctx context.Context, ch chan frames.Frame) error {
Expand Down
47 changes: 40 additions & 7 deletions kusto/internal/frames/v1/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"fmt"
"strings"

"github.com/Azure/azure-kusto-go/kusto/data/errors"
"github.com/Azure/azure-kusto-go/kusto/data/table"
"github.com/Azure/azure-kusto-go/kusto/data/types"
"github.com/Azure/azure-kusto-go/kusto/data/value"
"github.com/Azure/azure-kusto-go/kusto/data/errors"
"github.com/Azure/azure-kusto-go/kusto/internal/frames"
)

Expand Down Expand Up @@ -78,7 +78,7 @@ func (d *DataTable) unmarshalCols(m map[string]interface{}) error {
}
cn, ok := m[frames.FieldColumnName].(string)
if !ok {
return errors.E(d.Op, errors.KInternal, fmt.Errorf("DataTable.Columns had entry with .ColumnName set to a %T type", m[frames.FieldColumnName]))
return errors.E(d.Op, errors.KInternal, fmt.Errorf("DataTable.Columns(v1) had entry with .ColumnName set to a %T type", m[frames.FieldColumnName]))
}

// Note: The v1 backend doesn't seem to send the ColumnType most of the time. So,
Expand All @@ -89,13 +89,13 @@ func (d *DataTable) unmarshalCols(m map[string]interface{}) error {
if !ok {
dts, ok := m["DataType"].(string)
if !ok {
return errors.E(d.Op, errors.KInternal, fmt.Errorf("DataTable.Columns had entry with no .ColumnType set or .DataType "))
return errors.E(d.Op, errors.KInternal, fmt.Errorf("DataTable.Columns(v1) had entry with no .ColumnType set or .DataType "))
}
ct = types.Column(strings.ToLower(dts))
}

if !ct.Valid() {
return errors.E(d.Op, errors.KInternal, fmt.Errorf("DataTable.Columns had entry with .ColumnType set to a %T type", m[frames.FieldColumnType]))
ct, ok = translate[strings.ToLower(dts)]
if !ok {
return errors.ES(d.Op, errors.KInternal, "DataTable.Columns(v1) had entry with .DataType set to %q type, which is not supported", dts)
}
}

col := table.Column{
Expand Down Expand Up @@ -137,3 +137,36 @@ func (d *DataTable) unmarshalRows(m map[string]interface{}) error {
}

func (DataTable) IsFrame() {}

var translate = map[string]types.Column{
"bool": types.Bool,
"boolean": types.Bool,
"system.boolean": types.Bool,
"datetime": types.DateTime,
"date": types.DateTime,
"system.datetime": types.DateTime,
"dynamic": types.Dynamic,
"object": types.Dynamic,
"system.object": types.Dynamic,
"guid": types.GUID,
"uuid": types.GUID,
"uniqueid": types.GUID,
"system.guid": types.GUID,
"int": types.Int,
"int32": types.Int,
"system.int32": types.Int,
"long": types.Long,
"int64": types.Long,
"system.int64": types.Long,
"real": types.Real,
"double": types.Real,
"system.double": types.Real,
"string": types.String,
"system.string": types.String,
"timespan": types.Timespan,
"time": types.Timespan,
"system.timeSpan": types.Timespan,
"decimal": types.Decimal,
"system.data.sqltypes.sqldecimal": types.Decimal,
"sqldecimal": types.Decimal,
}
2 changes: 1 addition & 1 deletion kusto/internal/frames/v1/v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestDataTableUnmarshal(t *testing.T) {
"Columns": []interface{}{
map[string]interface{}{
"ColumnName": "TableId",
"DataType": "Int", // here is where I did it.
"DataType": "Int32", // here is where I did it.
},
map[string]interface{}{
"ColumnName": "Key",
Expand Down
2 changes: 1 addition & 1 deletion kusto/internal/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
package version

// Kusto is the version of this client package that is communicated to the server.
const Kusto = "0.1.0"
const Kusto = "0.1.1"

0 comments on commit 3d32447

Please sign in to comment.