timshannon / bolthold

BoltHold is an embeddable NoSQL store for Go types built on BoltDB
MIT License
643 stars 45 forks source link

AggregateResult.Max() does not use Comparer interface #81

Closed freakinhippie closed 4 years ago

freakinhippie commented 4 years ago

I'm attempting to use the FindAggregate() method to get the maximum value for each group. My field uses a semantic version string. I've implemented a custom type to store the semantic version in an effort to leverage the Comparer interface since the default string comparison will not work properly for a semantic version. Unfortunately, it seems that the AggregateResult.Max() method either doesn't use the Comparer interface, or I'm doing something wrong.

Here is some example code to illustrate my attempted implementation.

// example data in boltdb
[]MyType{
  MyType{
    Namespace: "team1",
    Version: SemVer{
      String: "0.2.2",
    },
  },
  MyType{
    Namespace: "team1",
    Version: SemVer{
      String: "0.2.11",
    },
  },
  MyType{
    Namespace: "team2",
    Version: SemVer{
      String: "1.15.18",
    },
  },
  MyType{
    Namespace: "team2",
    Version: SemVer{
      String: "1.2.12",
    },
  },
}

// SemVer holds a semantic version string
type SemVer struct {
  String string
}

// Compare implements bolthold.Comparer interface
func (t *SemVer) Compare(other interface{}) (int, error) {
  log.Println("Comparing current value to other value")
  if other, ok := other.(SemVer); ok {
    v, _ := version.NewVersion(t.String)
    o, _ := version.NewVersion(other.String)
    if v.Equal(o) {
      return 0, nil
    }
    if v.LessThan(o) {
      return -1, nil
    }
    return 1, nil
  }

  return 0, &bolthold.ErrTypeMismatch{Value: t, Other: other}
}

type MyType struct {
  Namespace   string    `boltholdIndex:"Namespace"`
  Version     SemVer    `boltholdIndex:"Version"`
}

// GetAggregateResults returns the max version for each namespace
func GetAggregateResults() {
  // ...open database, etc...

  result, err := store.FindAggregate(&MyType{}, nil, "Namespace")
  if err != nil {
    log.Println(err)
  }

  for i := range result {
    max := &MyType{}

    var namespace string

    result[i].Group(&namespace)
    result[i].Max("Version", max)
  }
}

I looked at compare_test.go to get an idea of how it is used and it doesn't have an example using the AggregateResult.Max(). It did have an example using the Query.Gt() method so I tried using that just to see if my Compare() method would log any output, but it did not.

I'm open to refactoring my implementation. I just need to be able to use the AggregateResult.* methods on a semantic version string.

timshannon commented 4 years ago

I wonder if it's due to the fact that the type in MyType is SemVer, but the comparer interface is defined on *SemVer.

You could try using &SemVer{} in your type and see if that works. I'll look into it as well.

Thanks,

freakinhippie commented 4 years ago

It seems you're correct. I changed the type to *SemVer and it is now calling my Compare method. There is still a problem with the internals of the function, but that's not related to bolthold.

Thank you!