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.

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

Improvements/add testcode modify cadinality #81

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
10 changes: 6 additions & 4 deletions builder/aggregation/cardinality.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package aggregation

// Cardinality Each individual element of the "fields" list can be a String or DimensionSpec.
// A String dimension in the fields list is equivalent to a DefaultDimensionSpec (no transformations).
type Cardinality struct {
Base
Fields []string `json:"fields,omitempty"`
ByRow *bool `json:"byRow,omitempty"`
Round *bool `json:"round,omitempty"`
Fields []interface{} `json:"fields,omitempty"`
ByRow *bool `json:"byRow,omitempty"`
Round *bool `json:"round,omitempty"`
}

func NewCardinality() *Cardinality {
Expand All @@ -18,7 +20,7 @@ func (c *Cardinality) SetName(name string) *Cardinality {
return c
}

func (c *Cardinality) SetFields(fields []string) *Cardinality {
func (c *Cardinality) SetFields(fields []interface{}) *Cardinality {
c.Fields = fields
return c
}
Expand Down
37 changes: 37 additions & 0 deletions builder/aggregation/cardinality_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package aggregation

import (
"encoding/json"
"github.com/grafadruid/go-druid/builder/dimension"
"github.com/grafadruid/go-druid/builder/extractionfn"
"github.com/stretchr/testify/assert"
"testing"
)

func TestCardinality(t *testing.T) {
cardinality := NewCardinality()
substringExtra := extractionfn.NewSubstring().SetIndex(0).SetLength(1)
extraction := dimension.NewExtraction().SetDimension("last_name").
SetOutputName("last_name_first_char").
SetExtractionFn(substringExtra)

cardinality.SetName("distinct_last_name_first_char").
SetFields([]interface{}{extraction}).SetByRow(true).SetRound(false)
expected := `{
"type": "cardinality",
"name": "distinct_last_name_first_char",
"fields": [
{
"type" : "extraction",
"dimension" : "last_name",
"outputName" : "last_name_first_char",
"extractionFn" : { "type" : "substring", "index" : 0, "length" : 1 }
}
],
"byRow" : true,
"round" : false
}`
cardinalityJSON, err := json.Marshal(cardinality)
assert.Nil(t, err)
assert.JSONEq(t, expected, string(cardinalityJSON))
}
27 changes: 27 additions & 0 deletions builder/aggregation/javascript_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package aggregation

import (
"encoding/json"
"github.com/stretchr/testify/assert"
"testing"
)

func TestJavascript(t *testing.T) {
javaScript := NewJavascript()
javaScript.SetName("sum(log(x)*y) + 10").
SetFieldNames([]string{"x", "y"}).
SetFnAggregate("function(current, a, b) { return current + (Math.log(a) * b); }").
SetFnCombine("function(partialA, partialB) { return partialA + partialB; }").
SetFnReset("function() { return 10; }")
expected := `{
"type": "javascript",
"name": "sum(log(x)*y) + 10",
"fieldNames": ["x", "y"],
"fnAggregate" : "function(current, a, b) { return current + (Math.log(a) * b); }",
"fnCombine" : "function(partialA, partialB) { return partialA + partialB; }",
"fnReset" : "function() { return 10; }"
}`
javaScriptJSON, err := json.Marshal(javaScript)
assert.Nil(t, err)
assert.JSONEq(t, expected, string(javaScriptJSON))
}
29 changes: 29 additions & 0 deletions builder/dimension/extraction_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package dimension

import (
"encoding/json"
"github.com/grafadruid/go-druid/builder/extractionfn"
"github.com/stretchr/testify/assert"
"testing"
)

func TestNewExtraction(t *testing.T) {
substringExtra := extractionfn.NewSubstring().SetIndex(0).SetLength(1)
extraction := NewExtraction().SetDimension("last_name").
SetOutputName("last_name_first_char").
SetExtractionFn(substringExtra)
expected := `{
"type": "extraction",
"dimension": "last_name",
"outputName": "last_name_first_char",
"extractionFn": {
"type": "substring",
"index": 0,
"length": 1
}
}`
extractionJSON, err := json.Marshal(extraction)
assert.Nil(t, err)
assert.JSONEq(t, expected, string(extractionJSON))

}
5 changes: 5 additions & 0 deletions builder/dimension/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ func (l *Lookup) SetOutputName(outputName string) *Lookup {
return l
}

func (l *Lookup) SetDimension(dimensionName string) *Lookup {
Copy link
Contributor

Choose a reason for hiding this comment

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

better name it dimension instead of dimensionName since it's what we've done everywhere (just like druid own codebase @JsonProperty("dimension") String dimension, https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/query/dimension/LookupDimensionSpec.java)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the detailed review, @jbguerraz

I completely agree with your thoughts. I hadn't thought of specifying it until I looked at the Java source code. I created a DimensionSpec interface and modified the Fields variable of cadinality. Please check if it matches your idea.

type DimensionSpec interface {
	Dimension
	GetDimension() string
	GetOutputName() string
	GetOutputType() types.OutputType
	GetExtractionFn() ExtractionFn // Deprecated
}

l.Base.SetDimension(dimensionName)
return l
}

func (l *Lookup) SetReplaceMissingValueWith(replaceMissingValueWith string) *Lookup {
l.ReplaceMissingValueWith = replaceMissingValueWith
return l
Expand Down
20 changes: 20 additions & 0 deletions builder/dimension/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dimension

import (
"encoding/json"
lookup2 "github.com/grafadruid/go-druid/builder/lookup"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -17,3 +18,22 @@ func TestDimensionLookup(t *testing.T) {
assert.Nil(t, err)
assert.JSONEq(t, expected, string(lookupJSON))
}

func TestNewLookup(t *testing.T) {
expected := `{
"type":"lookup",
"dimension":"dimensionName",
"outputName":"dimensionOutputName",
"replaceMissingValueWith":"missing_value",
"retainMissingValue":false,
"lookup":{"type": "map", "map":{"key":"value"}, "isOneToOne":false}
}`
mapFn := lookup2.NewMap().SetMap(map[string]string{"key": "value"}).SetIsOneToOne(false)
lookup := NewLookup().SetOutputName("dimensionOutputName").SetRetainMissingValue(false).
SetDimension("dimensionName").
SetReplaceMissingValueWith("missing_value").SetLookup(mapFn)

lookupJSON, err := json.Marshal(lookup)
assert.Nil(t, err)
assert.JSONEq(t, expected, string(lookupJSON))
}
2 changes: 1 addition & 1 deletion builder/extractionfn/substring.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package extractionfn

type Substring struct {
Base
Index int64 `json:"index,omitempty"`
Index int64 `json:"index"` // If omitempty is present, it disappears from json when index is 0
Length int64 `json:"length,omitempty"`
}

Expand Down