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

Introduce JsonPathExists function #2673

Closed
wants to merge 4 commits into from

Conversation

hahn-kev
Copy link

@hahn-kev hahn-kev commented Mar 3, 2023

Implements json_path_exists support via an extension function for #2668. Currently this version does not support the vars or silent parameter that the database function does.

I would love to implement the vars support especially otherwise there's some possibility for JSON path injection if a user defined variable is used to build the path expression, which seems very likely. I suppose the vars could be either a json string or a JsonElement like what was done with JsonContains but is there a risk of some kind of injection there too? Also it's quite difficult to work with either element or a json string in code.

As for implementing other json path methods this seems like the simplest to start with as it just returns a boolean, others that actually return json would be a lot harder to work with.

Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, please take a look at the comments below.

@hahn-kev hahn-kev force-pushed the raw-json-path-exists branch from b7b21ed to fdf2bec Compare April 3, 2023 08:57
@hahn-kev hahn-kev requested a review from roji April 3, 2023 09:43
@hahn-kev
Copy link
Author

@roji could you take a look at this again?

@hahn-kev
Copy link
Author

@roji anything blocking this that I can fix to get it merged in?

@roji
Copy link
Member

roji commented Jun 26, 2023

Sorry for not responding for so long... One reason for this was that I wanted to look at "exists in JSON" functionality in other databases, to take a wider view at this. It indeed turns out that all relational databases have basic support for this - I just opened dotnet/efcore#31136 with my findings. As a result, I think it would be better to introduce a relational-level API into EF Core, and then simply implement that in the PG driver; this would land us in a place where an application can execute such a query in a provider-agnostic way.

I'm sorry that this pushes back the work here even further... I do hope that the EF-side work (dotnet/efcore#31136) can happen for 8 - and you're very welcome to work on that as well if you're interested; please let me know if so.

Note that once that's done, I think it still makes sense to introduce a PostgreSQL-specific overload here for accepting the "vars" parameter (none of the other databases have this). However, it would need to be named the same as the simpler relational function we'll add in dotnet/efcore#31136, so that should be done first.

@hahn-kev
Copy link
Author

Ahh, that makes a lot of sense, thank you for thinking about the bigger scope of how this might apply, I hadn't thought about that and I think this makes much more sense. Bummer that it's going to delay this, but the work I did will still be useful some day at least.

As a side question, is there a way I could implement this for myself in a personal project?

@roji
Copy link
Member

roji commented Jun 30, 2023

As a side question, is there a way I could implement this for myself in a personal project?

Yep - it should be easy to use user-defined function mapping to map a .NET method to map jsonb_path_exists. Give it a try and let me know if you hit any trouble.

And thanks for understanding about this PR - I know it delays things, but it's important for us to keep a good cross-database story as much as we can. I'll do my best to push this into EF.

@roji roji closed this Jun 30, 2023
@gor8808
Copy link

gor8808 commented Sep 21, 2023

@hahn-kev If you still need to implement json path exists function for your personal project you can check out this sample: EFCore-PostgreSQL-Functions-Mapping

Hope it's useful

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

Successfully merging this pull request may close these issues.

3 participants