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

Should not have to wrap all WHERE values. #379

Open
veqryn opened this issue Aug 30, 2024 · 6 comments
Open

Should not have to wrap all WHERE values. #379

veqryn opened this issue Aug 30, 2024 · 6 comments

Comments

@veqryn
Copy link

veqryn commented Aug 30, 2024

Is your feature request related to a problem? Please describe.
It should not be necessary to wrap values that are being used for comparisons.
Here is what the code currently looks like, where we are wrapping id in Uint64():

id := 16
query := SELECT(
	Accounts.AllColumns
).FROM(
	Accounts,
).WHERE(
	Accounts.ID.EQ(Uint64(id)),
)

Describe the solution you'd like
Passing in regular values should be supported:

id := 16
query := SELECT(
	Accounts.AllColumns
).FROM(
	Accounts,
).WHERE(
	Accounts.ID.EQ(id),
)

Passing in a slice directly should be supported:

names := []string{"bob", "jane", "john"}
query := SELECT(
	Accounts.AllColumns,
).FROM(
	Accounts,
).WHERE(
	Accounts.Name.IN(names),
)
@go-jet
Copy link
Owner

go-jet commented Aug 31, 2024

EQ operator should be able to accept any expression not just a parameter. For instance:

Accounts.ID.EQ(Book.ID.ADD(Int(3))

Since golang doesn't allow method name overloading this is the only way.

Alternatively, we can add a new method for each type, but that would be just a marginally improvements:

Accounts.ID.EQ(Uint64(id)),
// vs
Accounts.ID.EQUint64(id),

Note just ( and ) disappears from the code. Not worth of adding 100 of new methods into library source code just to eliminate two brackets.

@veqryn
Copy link
Author

veqryn commented Sep 1, 2024

Why not have EQ accept any and then wrap the parameter in whatever function is appropriate for the value's type/interface?

@go-jet
Copy link
Owner

go-jet commented Sep 1, 2024

Then it wouldn't be a Type safe SQL builder anymore.

@veqryn
Copy link
Author

veqryn commented Sep 1, 2024

Could it accept a generics-constraint/interface that includes both the correct Expression type, and also the base primitives?

@go-jet
Copy link
Owner

go-jet commented Sep 4, 2024

The last time I tried with generics, the issue was that it was not possible to have generic method on a type. The whole type has to be generic. Maybe that has changed in the meantime. Or maybe there is some other workaround.
It is possible to redefine some base type and then make it implement all Expression methods. But still when you get base type from http request you will have to cast it into new type. Which will look basically the same as what we currently have.

@kblomster
Copy link
Contributor

The last time I tried with generics, the issue was that it was not possible to have generic method on a type. The whole type has to be generic. Maybe that has changed in the meantime.

It has not changed, and as far as I've read it is unlikely to change in the future too. Some technical constraints, IIRC.

FWIW I like the current interface, it makes it very explicit when you're passing in something that will be compiled into a SQL parameter vs when you're passing some other type of expression.

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

No branches or pull requests

3 participants