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

Add support for database functions #224

Open
lafriks opened this issue Oct 12, 2021 · 10 comments
Open

Add support for database functions #224

lafriks opened this issue Oct 12, 2021 · 10 comments

Comments

@lafriks
Copy link
Contributor

lafriks commented Oct 12, 2021

Would be nice to have ability to specify some common functions in filters and default values.

At least few I can think of:

  • Current date and time (current_timestamp, current_date, current_time)
    • MSSQL - getdate(), convert (date, getdate()), convert (time, getdate())
  • upper/lower - same for all currently supported
  • substring
    • PostgreSQL - substring ( string text [ FROM start integer ] [ FOR count integer ] )
    • MSSQL, sqlite3, MySQL - substring ( expression, start, length )
  • nullif/coalesce - same for all currently supported
  • least/greatest
    • sqlite3 - min, max
    • MSSQL, PostgreSQL, MySQL - least, greatest

Probably some others and have adapter to build them as on different databases this can be different

@Fs02
Copy link
Member

Fs02 commented Oct 12, 2021

Currently, the only possible way to do this is by using Fragment in the query

This could be a nice addition, maybe we need to think REL API (how to integrate it to existing api) to support them first, do you have anything in mind?

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

Probably need to add something like FilterFunctionOp where Field would be from some kind of function enum and function arguments would be in Inner. This way it would be possible to have something like:

// function could be subpackage like where currently is
function.Substring(rel.FilterField("field_name"), rel.FilterValue(1), rel.FilterValue(10))

rel.FilterField:

	return FilterQuery{
		Type:  FilterFieldOp,
		Field: name,
	}

rel.FilterValue:

	return FilterQuery{
		Type:  FilterValueOp,
		Value: value,
	}

rel.FilterFunction:

	return FilterQuery{
		Type:  FilterFunctionOp,
		Field: name,
		Inner: args,
	}

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

and in builder it would be needed some kind of callback like for column map that would provide arguments:

  • Function name from enum
  • Arguments as []string (already preprocessed and built filter expressions)

and callback function would return built SQL to be written in buffer

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

btw rel.FilterField can be useful on it's own for example for:

where.Not(rel.FilterField("bool_column"))

while this is possible also with rel.FilterFragment provided column name will not be escaped/quoted

@Fs02
Copy link
Member

Fs02 commented Oct 12, 2021

where.Not(rel.FilterField("bool_column"))

I think specifying rel.FilterField in every field will make the api too verbose and not compact.

while this is possible also with rel.FilterFragment provided column name will not be escaped/quoted

this is actually possible, using fragment or not, just add ^ character at the beginning of field name, and it won't be escaped (it's a hidden feature 😅 )
https://github.com/go-rel/sql/blob/36066d80a50a7aaa9a4532df23de99e254a5310c/builder/buffer.go#L16-L17

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

But it will not escape at all rel.FilterFragment: https://github.com/go-rel/sql/blob/36066d80a50a7aaa9a4532df23de99e254a5310c/builder/filter.go#L45 it would be useful to HAVE a way to escape with rel.FilterField

@lafriks
Copy link
Contributor Author

lafriks commented Oct 12, 2021

while I agree this is verbose but it's most easiest way to implement and have flexibility that this provides. As you could have also complex expressions like:

function.Upper(
    function.Substring(
        rel.FilterField("field_name"),
        rel.FilterValue(1),
        function.Least(rel.FilterField("some_other_field), rel.FilterValue(10)),
    ),
)

@Fs02
Copy link
Member

Fs02 commented Oct 14, 2021

I did something similar in the previous reincarnation of this library, and the code get messy easily
https://github.com/Fs02/grimoire/blob/af0b7afd317048419836fafc91f0caf3323b9f8e/query_test.go#L74-L77

I can't always control and suggest how other use the API, so I learn that I need to limit and teach it by design
maybe we can implement it better when go generics support released it later without sacrificing readability

@lafriks
Copy link
Contributor Author

lafriks commented Oct 18, 2021

that might be an option

@dranikpg
Copy link
Contributor

This could be a nice addition, maybe we need to think REL API (how to integrate it to existing api) to support them first, do you have anything in mind?

Do I understand correctly that the main problem is maintaing the same type safe (more or less) API?

Now that generics are out, they won't help that much, unfortunately. If we're striving for supporting something like rel.Select(funcs.Count("id"), funcs.Max("price"), "just_field"), then this is just not possible with anything except ...interface{}

However in case of non-variadic functions, we can parameterize by [T string | relDbFunc ] and accept both.
This allows to introduce database functions to functions like Eq(string, interface{}) which becomes Eq(string | dbFunc, interface{}), so we get rel.Eq(funcs.Upper("name"), "USER") and rel.Eq("name", "user")

The multi-argument cases only could be handled with ...interface{} or by calling twice .Select("string", "values").Select(funcs.Lower("field")), because all variadic argument have to be either string or relDbFunc (some improvised type for now)

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