search

go-jet / jet

Type safe SQL builder with code generation and automatic query result data mapping
Apache License 2.0
2.52k stars 118 forks source link

Public APIs expose internal types #152

Closed anuraaga closed 1 year ago

anuraaga commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

There are some public APIs that expose internal tyeps, for example Table.INSERT

https://github.com/go-jet/jet/blob/master/mysql/table.go#L10

jet.Column is internal.

Expected behavior

All public APIs should only use public types. For example, it doesn't seem to be possible for a user to pass in a slice of columns with the current signature.

go-jet commented 2 years ago

To pass a custom column list to insert or update query, you can use ColumnList:

https://github.com/go-jet/jet/blob/6babd43e3a1af2a7bdff78c32423d0c5e601a644/tests/postgres/update_test.go#L288-L294

I'll add this to wiki.

anuraaga commented 2 years ago

Thanks @go-jet - would it be possible to use that type in the function definitions? I think it can be confusing to use the API when it references the internal instead of public type there.

go-jet commented 2 years ago

We can't use ColumnList in definition because UPDATE should work with single Column, comma separated list of columns and ColumnList. Now, we could expose jet.Column, but I don't see how this type would be ever used.

anuraaga commented 2 years ago

@go-jet I found another API that seems to use public types, Statement.Rows() returns jet.Rows (I can't pass Statement either for similar reason). I can't pass this to another function because jet.Rows is internal. Is there an alternative type for this? In general I suspect there will be less sharp edges if the public API was audited to remove all the internal types somehow.

go-jet commented 2 years ago

Yeah, it seems jet.Rows is not exposed. Contributions are welcomed. Statement is exposed here.

amanbolat commented 1 year ago

@go-jet Wouldn't it be better to expose all the internal APIs? If I understand it correctly, doing so would enable the users of this library to write their own implementations of given interfaces, rather than relying on and waiting for some types being implemented in go-jet.

galexrt commented 1 year ago

I think I'm running into this with some other types.

While trying to create a simple SELECT query that takes in a variable to equal check against the lastname field the internal jet package is imported. This prevents the code from working at all due to the following error:

app.go:10:2: use of internal package github.com/go-jet/jet/v2/internal/jet not allowed
stmt := Users.SELECT(Users.AllColumns).FROM(Users).WHERE(Users.Lastname.LIKE(jet.String("")))

Do you have any way around that besides fixing this in the jet repo?

houten11 commented 1 year ago

String is already exported. You are importing the wrong package. You need to import a dialect-specific package. For instance: github.com/go-jet/jet/v2/postgres

galexrt commented 1 year ago

String is already exported. You are importing the wrong package. You need to import a dialect-specific package. For instance: github.com/go-jet/jet/v2/postgres

Thanks for the help! I just realized that as well as I don't use the dot imports as shown in the READMe at the moment.

rowland66 commented 1 year ago

I have run into the same problem. I might try creating a pull request unless some is working on this.

go-jet commented 1 year ago

Rows is now exposed on master branch.

go-jet commented 1 year ago

Fixed with Release v2.10.1.