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

Cosmos: review cast/convert handling #34982

Open
roji opened this issue Oct 25, 2024 · 5 comments · May be fixed by #35000
Open

Cosmos: review cast/convert handling #34982

roji opened this issue Oct 25, 2024 · 5 comments · May be fixed by #35000

Comments

@roji
Copy link
Member

roji commented Oct 25, 2024

In general, the logic in CosmosSqlTranslatingEV.VisitUnary for ExpressionType.Convert. See e.g. test Equality_operator_int_to_long.

@ChrisJollyAU
Copy link
Contributor

@roji Would this be along the lines of a superset of the issue I picked up the other day with the string concatenation (#34963)

Also, I've tried looking but can't find the test you mentioned

@roji
Copy link
Member Author

roji commented Oct 25, 2024

Also, I've tried looking but can't find the test you mentioned

Yes, that test is something I'm still working on in a branch. Here it is:

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Equality_operator_int_to_long(bool async)
{
	long arg = 10248;

	return AssertQuery(
		async,
		ss => ss.Set<Order>().Where(o => o.OrderID == arg));
}

@roji
Copy link
Member Author

roji commented Oct 25, 2024

Would this be along the lines of a superset of the issue I picked up the other day with the string concatenation

And yeah, it could definitely be related...

@ChrisJollyAU
Copy link
Contributor

btw, had an idea based on what I have done previously

I can get this sql and it passes

@__arg_0='10248'

SELECT VALUE c
FROM root c
WHERE ((c["$type"] = "Order") AND (c["OrderID"] = @__arg_0))

Am I correct in thinking that Cosmos has some implicit conversions, as I couldn't find much in terms of an explicit cast

@ChrisJollyAU ChrisJollyAU linked a pull request Oct 28, 2024 that will close this issue
@ChrisJollyAU
Copy link
Contributor

@roji Submitted a PR that has a lot of improvements and lights up a bunch of tests that were failing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants