Collection update

[DeanPDX]

Further coverage on collection

• Collection: Support for updateOne.
• Convert ALL options to builders with pointer values for options.
• Export ptr package (previously internal). Useful now that all options are pointers.

[toptobes]

Could be worth returning an error in this case instead of silently wiping the other which could be arguably unexpected/surprising behavior?

[DeanPDX]

That's an interesting idea. I thought I'd wipe the other out just as a convenience to the user. The OTHER option is: we do nothing and let the API return an error if it wants to (I didn't actually test it to see if the API complains if you populate both; I just assumed it did).

[toptobes]

I'm 99.9% sure it complains. I'm fine either way (with having either the client or the API complain), I'm just hesitant about silently messing with the options. Pros and cons 🤷

[DeanPDX]

I made this part of IndexingOptions's Validate implementation. And added tests to make sure it works.

[toptobes]

Hm I'm conflicted... on one hand I like the symmetry and the intuitiveness, but on the other hand any potential errors are being swallowed... wdyt?

[DeanPDX]

Yeah... my THOUGHT was that we would want to return errors from the final MergeOptions. But that doesn't check child options by default - and doing so is kind of ugly/manual. Here's what that looks like with a few helper funcs:

// ValidateIfSet calls Validate on v if it is non-nil.
func ValidateIfSet[T Validator](v *T) error {
    if v == nil {
        return nil
    }
    return (*v).Validate()
}

// Just a helper to return first error in variadic input to prevent a bunch of if err != nil.
func firstErr(errs ...error) error {
    for _, err := range errs {
        if err != nil {
            return err
        }
    }
    return nil
}

func (o CreateCollectionOptions) Validate() error {
    // Here's what I don't like: we need to remember to add child Validators to this list
    return firstErr(
        ValidateIfSet(o.DefaultId),
        ValidateIfSet(o.Vector),
        ValidateIfSet(o.Indexing),
        ValidateIfSet(o.Lexical),
        ValidateIfSet(o.Rerank),
    )
}

It's not terrible; and we could just catch the "forgot to call validate on children" problem with unit tests. Or - I could look into using codegen to create a partial like so:

// options_validate_gen.go (generated) — never edit this
func (o CreateCollectionOptions) validateChildren() error {
	if err := ValidateIfSet(o.DefaultId); err != nil {
		return err
	}
	if err := ValidateIfSet(o.Vector); err != nil {
		return err
	}
	// ...
	return nil
}

And then, the only potential "gotcha" is that parents with child validators would need to remember to call that:

func (o CreateCollectionOptions) Validate() error {
	if o.Name == "" {
		return errors.New("name is required")
	}
	return o.validateChildren()
}