#195 - Add Support for Clickhouse

[AidanKenney]

References

Issue(s):#195

Description

Add support for Clickhouse driver using GORM

[coveralls]

Pull Request Test Coverage Report for Build 10066191359

Details

• 37 of 45 (82.22%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.1%) to 97.36%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
database/dialect/clickhouse/clickhouse.go	0	3	0.0%
validation/unique.go	37	42	88.1%

Totals
Change from base Build 10061155490:	-0.1%
Covered Lines:	6233
Relevant Lines:	6402

---

💛 - Coveralls

[System-Glitch]

Thank you very much for your contribution! Can you check if the Exists/Unique validator works with clickhouse please?
I don't think it will because of the VALUES syntax. It's not a problem for now, another issue will be created if needed.

[AidanKenney]

Sure thing @System-Glitch! And I haven't been able to get the clickhouse dialect to work in unique_test.go yet, I think the tests will need a bit of refactoring for it to work. Do you want me to try to get that working before moving ahead with this?

[System-Glitch]

Sure thing @System-Glitch! And I haven't been able to get the clickhouse dialect to work in unique_test.go yet, I think the tests will need a bit of refactoring for it to work. Do you want me to try to get that working before moving ahead with this?

As you prefer. If you're not willing to do it, this can be done in another issue later on.

[AidanKenney]

Let me take another shot at it, I'll follow up if I'm not able to get anywhere

[System-Glitch]

Can you add a test for the transform function with clickhouse please?

I'm sorry to request so many changes, properly supporting clickhouse for this validator is trickier than I expected.

[System-Glitch]

This can be done once outside the loop. It would be cleaner in my opinion:

var zeroVal T
paramType, ok := clickhouseTypes[reflect.TypeOf(zeroVal)]

You should check if the value is supported, otherwise add an error to the validation context if no Transform function is provided.

transformedValue is a gorm.Expr so your reflect call here will never work if you provide a transform function.

[AidanKenney]

So if the value is not supported within the clickhouseTypes map, but there is a Transform function provided, will we be able to determine what paramType should be?

[System-Glitch]

Transform is an option for special cases. For example for Postgres it allows to do the following:

UniqueArray("table", "column", func(val int64) clause.Expr {
	return gorm.Expr("?::bigint", val)
})

UniqueArray("table", "column", func(val string) clause.Expr {
	return gorm.Expr("LOWER(?)", val)
})

So in the case of clickhouse, this would allow developers to cast raw values into special types that we don't support directly.

[AidanKenney]

@System-Glitch No worries! Thank you for all these diligent reviews. Pushed up a few changes, please let me know if this is what you had in mind.

I might not be understanding you on the role of the Transformer -- is there a way we can use the Transformer to derive the Clickhouse types we need to indicate, even if the input's Go type has no match?

[System-Glitch]

👍🏻 This is exactly what I had in mind. I may tweak the error message a bit after this is merged.

[AidanKenney]

Thanks @System-Glitch, looks like it needs one more run before I can merge.

[System-Glitch]

Merged. Thank you very much! 🎉