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

Functions: Allow specifying in metadata if a function propagates nulls #13718

Open
divega opened this issue Oct 22, 2018 · 5 comments
Open

Functions: Allow specifying in metadata if a function propagates nulls #13718

divega opened this issue Oct 22, 2018 · 5 comments

Comments

@divega
Copy link
Contributor

divega commented Oct 22, 2018

I have the vague idea that we have discussed this before with @pmiddleton but I couldn't find an issue for it, and today the need came up in triage to differentiate functions that return null when one of the arguments is null (most functions) from the ones that don't, e.g. COALESCE, IFNULL, etc.

The idea of this issue is that instead of hardcoding the assumption and maintaining a list of exceptions, we can just have a flag in the metadata of each function to indicate that it propagates null (probably defaulting to indicate that it does).

This could be relevant to the issue on comparision of function results at #13642 that @maumar is looking at, and to some of the Boolean functions in SpatiaLite that @bricelam is testing in #13695.

@pmiddleton
Copy link
Contributor

@divega - We have never talked about this that I remember. We have discussed tracking whether or not functions are "built in" vs user defined. That is needed so we can properly escape function calls in Sql Server and PostgreSQL. See #12757. Maybe this is what you are thinking of?

If your issue is only with builtin functions for Sql Server then for now you can expand SqlFunctionExpression and add a new field to track this attribute of functions. It's not ideal because you have to map the MethodCallExpression to SqlFunctionExpression to get to the data, and depending on where you are in the code you might not be able to make that translation. You may need to make the translation call and pass around the result so you have it where you need it.

For 3.0 we really need to create a function metadata store. This store will contain all of the metadata about the mapped functions and will do this for all builtin, extension (Include, AsNoTracking, ...), and user defined functions. This store can then be attached to the database model and used during query translating. This way everything is where we need it, when we need it, and in the form we need it in.

Since we will be fully translating the raw expression tree all functions can now be treated the same. No more having these 3 function types be treated differently by the code base.

cc: @smitpatel @ajcvickers

@divega
Copy link
Contributor Author

divega commented Oct 23, 2018

For 3.0 we really need to create a function metadata store

Thanks @pmiddleton. I think this is the main thing I had in my mind as something we have discussed that could help.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Oct 24, 2018
@ajcvickers
Copy link
Member

@maumar to switch COALESCE special casing to this.

@smitpatel smitpatel removed this from the 3.0.0 milestone Dec 3, 2018
@divega divega added this to the 3.0.0 milestone Dec 7, 2018
@divega
Copy link
Contributor Author

divega commented Dec 7, 2018

Triage: Adding this back to 3.0. We need to think in general about how null propagation is going to be treated on functions in the new query pipeline.

@smitpatel
Copy link
Member

@maumar to conclude this one.

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

5 participants