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

Handle postgres array #7330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kenjihikmatullah
Copy link

@kenjihikmatullah kenjihikmatullah commented Jan 1, 2025

  • Do only one thing
  • Non breaking API changes
  • Tested

Basically, this is the continuation of the changes on postgres driver part: go-gorm/postgres#296
and must not be merged before postgres driver changes is merged

Hi @jinzhu and team, please review when you have a moment. Any feedbacks are welcome. Thanks!

What did this pull request do?

Handle postgres array types

User Case Description

Currently, when we we have column with array type in postgres database, we need to use or create custom type in our go program so that our program (with gorm) can map the type from/to database.

For example, we can't do this by default, because gorm will throw something like unsupported data type: &[]

type Country string

type Player struct {
	...
	Country               []Country `gorm:"type:text[]"`
}

we need to create custom type, for instance:

type CountryArray []Country

func (a *CountryArray) Scan(src interface{}) error {
	var asBytes []byte
	switch v := src.(type) {
	case []byte:
		asBytes = v
	case string:
		asBytes = []byte(v)
	default:
		return errors.New("Scan source was not []bytes or string")
	}

	str := string(asBytes)
	parsed := strings.Trim(str, "{}")
	split := strings.Split(parsed, ",")

	*a = make(CountryArray, 0)
	for _, s := range split {
		if s == "" {
			continue
		}
		day, err := value_object.NewDayOfWeekFromString(s)
		if err != nil {
			return err
		}
		*a = append(*a, *day)
	}

	return nil
}

func (a CountryArray) Value() (driver.Value, error) {
	strArr := make([]string, len(a))
	for i, day := range a {
		strArr[i] = day.String()
	}
	return "{" + strings.Join(strArr, ",") + "}", nil
}

Then, use it in the model

type Country string

type Player struct {
	...
	Country               CountryArray `gorm:"type:text[]"`
}

By this PR, we can just implement like this, without having to use/create custom type for each array column

type Country string

type Player struct {
	...
	Country               []Country `gorm:"type:text[]"`
}

to achieve this, we just need to enable it via config

postgres.Config{
	...,
	EnableArrayHandler:   true,
}

Compatibility and Tests

The logic of array handling is placed specifically on postgres driver so that gorm core remains clean, extensible, and database-agnostic. Since a new gorm core test case depends on new changes on postgres driver (which is new config attribute), we need to make sure that changes on postgres driver must be merged before changes on gorm core. However, it is fine if they are not merged simulatenously since this is backward compatible

Tests New gorm core using new postgres driver Screenshot 2025-01-01 at 16 00 46

Existing gorm core using new postgres driver
Screenshot 2025-01-01 at 16 04 26

@jinzhu
Copy link
Member

jinzhu commented Jan 12, 2025

I think we can provide a method for the Dialector to determine whether the driver directly supports the type of a given field. This approach would make it more flexible.

@kenjihikmatullah
Copy link
Author

I think we can provide a method for the Dialector to determine whether the driver directly supports the type of a given field. This approach would make it more flexible.

@jinzhu thanks for the feedback. Do you mean like this?
Screenshot 2025-01-13 at 09 56 53
Screenshot 2025-01-13 at 09 57 51

If so, I am still not sure on couple of things:

  1. Since we add new method to Dialector interface, means all implementation must implement it. How can we make the changes not breaking?
  2. Where would this new method be called?

Your feedbacks would be very appreciated, 谢谢!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants