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

Feature: add dialect dependent functions #596

Open
zepatrik opened this issue Oct 12, 2020 · 1 comment
Open

Feature: add dialect dependent functions #596

zepatrik opened this issue Oct 12, 2020 · 1 comment
Labels
proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue
Milestone

Comments

@zepatrik
Copy link
Contributor

Description

There are some basic build-in SQL functions that are not supported by every database in the same way. One example is the version() function. Sqlite does not support it but has sqlite_version(). Another example is now() wich is also not supported by sqlite, instead one has to use CURRENT_TIMESTAMP there.

Expected Behavior

I would like pop to handle these cases for me. One idea I had was to export some constants like pop.Now or pop.Version that one can insert into the query and that would be filled in by the driver accordingly.
E.g.:

c.RawQuery("SELECT ? as version", pop.Version).First(&v))

Actual Behavior

Currently I have to do it similar to:

versionFunc := "version()"
if c.Dialect.Name() == "sqlite3" {
	versionFunc = "sqlite_version()"
}
/* #nosec G201 - versionFunc is an enum */
require.NoError(t, c.RawQuery(fmt.Sprintf("SELECT %s as version", versionFunc)).First(&v))

I can work on this if it is desired to have.

@aeneasr
Copy link
Member

aeneasr commented Oct 12, 2020

? is unfortunately reserved for escaped SQL arguments (on a driver level) which makes the proposal difficult to implement. Alternatively, we could expose an interface of "generic" SQL functions such as NOW() or version in a common interface which you could use with fmt.Sprintf. Question is if there is enough overlap in these generic functions to warrant such an interface or if there are really just a few (e.g. NOW()) where this could be used.

@sio4 sio4 added proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue labels Sep 20, 2022
@sio4 sio4 added this to the Proposal milestone Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue
Projects
None yet
Development

No branches or pull requests

3 participants