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

Custom Column Type #273

Open
amanbolat opened this issue Sep 16, 2023 · 3 comments
Open

Custom Column Type #273

amanbolat opened this issue Sep 16, 2023 · 3 comments

Comments

@amanbolat
Copy link

amanbolat commented Sep 16, 2023

Is your feature request related to a problem? Please describe.

Currently, go-jet generator supports only a limited type of columns. For example, for uuid, text or bytea columns only the postgres.ColumnString type is used.

The issues I'm facing at the moment is I cannot derive the real type of the column, and therefore it's impossible to dynamically build WHERE clause.

Example SQL table:

CREATE TABLE table1 (
    id              UUID PRIMARY KEY,
    value           TEXT
);

Now, I have a function that creates postgres.BoolExpression based on the column type:

func (b JetFilterBuilder) buildEqExpression(col postgres.Column, value string) (postgres.BoolExpression, error) {
	switch typedCol := col.(type) {
	case postgres.ColumnString:
		return typedCol.EQ(postgres.String(value)), nil
        // Skip rest of the code.
}

If I use table.Table1.Id column and pass some text value, it will create SQL like this one:

WHERE id = 'value'::text

Describe the solution you'd like

Options

Option 1

Enable the user to use custom column type during the generation. Right now, I think it's not possible using the generator.
Example of my attempt:

return tableBuilder.UseColumn(func(column metadata.Column) template.TableSQLBuilderColumn {
	if column.DataType.Name == "uuid" {
		return template.TableSQLBuilderColumn{
			Name: column.Name,
			Type: "UUID",
		}
	}

	return template.DefaultTableSQLBuilderColumn(column)
})

Will generate:

type table1 struct {
	postgres.Table

	//Columns
	id               postgres.ColumnUUID // <---------- type doesn't exists.
|

Would be nice to have the same ability to pass a type, as it's done for model field type.

Option 2

Add real column type and other metadata to the Column instance. However, it might increase the size of the column object.

My current solution

I run a tool that will parse the AST tree of the generated file and change the column type to the custom one.

Custom column type example:

type ColumnUUID struct {
	postgres.ColumnString
}

func UUIDColumn(val string) ColumnUUID {
	return ColumnUUID{postgres.StringColumn(val)}
}

Final goal

Assume I had the ability to introduce my custom column, my code would look like:

func (b JetFilterBuilder) buildEqExpression(col postgres.Column, value string) (postgres.BoolExpression, error) {
	switch typedCol := col.(type) {
	case postgres.ColumnString:
		return typedCol.EQ(postgres.String(value)), nil
	case custom_package.ColumnUUID:
                uuidVal := parseUUID(value)
		return typedCol.EQ(postgres.UUID(uuidVal)), nil
        // Skip rest of the code.
}

Thoughts

Maybe I missed something, and it's still possible to achieve my goal without any changes to the current version of go-jet, yet I haven't found one.

When I faced the issue, it seemed to me that there is no value in not exposing jet internal interfaces and structs, that could make other developer's life much easier, by extending the library with custom types and other features.

@go-jet
Copy link
Owner

go-jet commented Sep 18, 2023

Hi @amanbolat,
As I see it, the issue is that postgres.String adds a mandatory ::text cast. And that's unfortunate because behind postgres.String, there can be any of the currently unsupported types (like json, uuid, etc...). The simplest solution would be to have a string literal function without sql cast. Luckily, there is one already postgres.UUID, so a workaround in your case would be:

func (b JetFilterBuilder) buildEqExpression(col postgres.Column, value string) (postgres.BoolExpression, error) {
	switch typedCol := col.(type) {
	case postgres.ColumnString:
		return typedCol.EQ(postgres.UUID(value)), nil   //<-- Use UUID constructor for both text and uuid columns
        // Skip rest of the code.
}

I like both of your ideas, but I think once we add support for missing types, there would be much need for such a feature.

@amanbolat
Copy link
Author

Hi @go-jet,

Thank you for the suggestion, yet I need an explicit check in case of UUID as in my example:

case custom_package.ColumnUUID:
    uuidVal := parseUUID(value)
    return typedCol.EQ(postgres.UUID(uuidVal)), nil

The same will be required for JSON and JSONB types for my use case.

My current solution of extending the generator with AST parsing and substitution works as expected, but it feels as a hack.

Nevertheless, I suppose it's hard to keep the library up to date with all the possible types and different databases. Therefore, exposing internal types and interfaces seems a better solution in a long term.

I would like to make some contribution and open the PRs, however I'm still trying to understand all the internal architecture to make sure that my requirements do align with your vision.

@go-jet
Copy link
Owner

go-jet commented Sep 18, 2023

Aha, I see. In that case we can go with option 1. It should be a small change. We can add a new field into TableSQLBuilderColumn something like TypeTemplate jet.ColumnExpression . If TypeTemplate is nil, Type field is used.
As other code of interest take a look at process.go processTableSQLBuilder and file_templates.go tableSQLBuilderTemplate.

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

No branches or pull requests

2 participants