Skip to content

NpgsqlJsonPocoTranslator.TranslateMemberAccess sets PgJsonTraversalExpression.ReturnsText to true during ordering operation #3457

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

Closed
dheardal opened this issue Feb 7, 2025 · 4 comments

Comments

@dheardal
Copy link

dheardal commented Feb 7, 2025

We are trying to order by a numerical column within a JSON POCO document.

Looking at the queries that are generated the property access within the order by evaluates to column ->> value. The problem with this is that ->> is the json text operator meaning when the value is numerical the ordering is incorrect.

Given this is within an order by and we do not need to bring the result back we would expect the operator to be -> in which case postgres would use the native type and ordering of the numerical values would be correct.

Looking at the implementation of NpgsqlJsonPocoTranslator.TranslateMemberAccess it seems the problem is that it always sets PgJsonTraversalExpression.ReturnsText to true which means it will always use the ->> operator instead of the -> operator.

@dheardal dheardal changed the title NpgsqlJsonPocoTranslator.TranslateMemberAccess always sets PgJsonTraversalExpression.ReturnsText to false NpgsqlJsonPocoTranslator.TranslateMemberAccess sets PgJsonTraversalExpression.ReturnsText to false during ordering operation Feb 7, 2025
@dheardal dheardal changed the title NpgsqlJsonPocoTranslator.TranslateMemberAccess sets PgJsonTraversalExpression.ReturnsText to false during ordering operation NpgsqlJsonPocoTranslator.TranslateMemberAccess sets PgJsonTraversalExpression.ReturnsText to true during ordering operation Feb 7, 2025
@roji
Copy link
Member

roji commented Feb 7, 2025

Can you please provide a minimal, runnable console program that shows an error happening?

@dheardal
Copy link
Author

So the above happened when we were using the legacy POCO method and a dynamic property.

I have converted to use the EF JSON support, which means i can't use a dynamic property. In this case I've noticed that the sql it is generating looks like this (CAST(v1.value ->> 'BoolValue' AS boolean)). However if the query was to be generated using -> so v1.value -> 'BoolValue' then there would be no need for the cast.

I appreciate the legacy POCO method is deprecated so happy not to close the original issue, however the cast question remains.

@roji
Copy link
Member

roji commented Feb 10, 2025

However if the query was to be generated using -> so v1.value -> 'BoolValue' then there would be no need for the cast.

I don't think that's true; the -> operator returns a jsonb, not a bool:

SELECT pg_typeof('{ "a": true }'::jsonb -> 'a'); -- jsonb
SELECT pg_typeof(CAST('{ "a": true }'::jsonb -> 'a' as bool)); -- boolean

The better way forward here in general is to use the new JSON_VALUE function (#3304), though that's PG17 only.

BTW even for legacy POCO mapping, I'm not aware of this problem happening - if you put together a minimal repro I can take a look. I'm not sure why legacy POCO vs. EF JSON would behave differently here in general.

@dheardal
Copy link
Author

I think it does have the same behaviour, the only difference being legacy POCO can deal with a dynamic property but EF cannot, and with the dynamic property you do not get the CAST.

I think there may be a potential improvement here when EF supports weakly-typed properties (dotnet/efcore#28871 / dotnet/efcore#29825) that property access within an orderby should return the jsonb, so you would get ORDER BY object-> 'weak_property' not ORDER BY object->> 'weak_property' so that postgres can order numerical values correctly

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 11, 2025
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

2 participants