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

Unnecessary where clause logic grouping and ignored IN queries #26322

Open
sam-wheat opened this issue Oct 12, 2021 · 5 comments
Open

Unnecessary where clause logic grouping and ignored IN queries #26322

sam-wheat opened this issue Oct 12, 2021 · 5 comments

Comments

@sam-wheat
Copy link

Questions

1.) In the Linq below only two of the AND clauses are grouped with parenthesis however the generated SQL nests each AND clause (except the last one for unknown reasons). I can't say this will generate incorrect results however it does make the query much more difficult to read and use for debugging purposes. Is this the expected behavior?

2.) The lookups using the string arrays are ignored. The expected SQL is something like ...CASE WHEN left(substring(rn.nativeaddress, 8, LEN(rn.nativeaddress) - 8), CHARINDEX('.', substring(rn.nativeaddress, 8, LEN(rn.nativeaddress) - 8))- 1) IN ('64','65','66','67','68','69','70','71','72','73','74','75','76','77','78','79') THEN 'Vendor1'.... I can't make sense of the generated SQL. Arguably this lookup can be done client side but it's a bit more efficient for me I can just get back a ready-to-use object from the server.

If either of these questions warrant further investigation please let me know and I will put together a runnable example to reproduce it.

Thanks.

Linq

DateTime now = DateTime.UtcNow;
string[] shopSerialNumbers = { "11", "53098" };
string[] vendor1Address = { "64", "65", "66", "67", "68", "69", "70", "71", "72", "73", "74", "75", "76", "77", "78", "79" };
string[] vendor2Address = { "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15" };
string[] vendor3Address = { "172" };
decimal[] hardwareVersions = { 1m, 3m, 0m };
			
var query = from acr in db.OwceCommRates
join jk in db.InterrogationJobs on acr.JobKey equals jk.JobKey
join rn in db.RelayNodes on acr.CellRelayId equals rn.CellRelayId
join m in db.Nodes on rn.NodeKey equals m.RelayNodeKey.Value into nodesGraph
from node in nodesGraph.DefaultIfEmpty()
let tmp = string.IsNullOrEmpty(rn.NativeAddress) ? null : rn.NativeAddress.Substring(8, rn.NativeAddress.Length - 8) 
let carrierAddress = string.IsNullOrEmpty(tmp) ? null : tmp.Substring(0, tmp.IndexOf('.'))
where
node != null
&& jk.SubmittedTime.Value.Date > now.Date
&& jk.SubmittedTime.Value.Hour < 12
&& jk.JobStatus == "W"
&& (acr.TotalEndpoints - acr.SuccessfulEndpoints) > 30
&& acr.CellRelayId > 0
&& (!shopSerialNumbers.Contains(rn.SerialNumber))
&& (hardwareVersions.Contains(node.HardwareVersion.Value) || Convert.ToInt32(node.SerialNumber) > 9999999)
&& node.Registered == 1m
select new RelayView
{
	JobKey = acr.JobKey,
	InterrogationDate = jk.StartTime.Value,
	FAR_ID = acr.CellRelayId,
	CellMaster = node.SerialNumber,
	NativeAddress = rn.NativeAddress,
	MCHInstanceId = acr.MchinstanceId,
	Carrier =   vendor1Address.Contains(carrierAddress) ? "Vendor1" : 
				vendor2Address.Contains(carrierAddress) ? "Vendor2" : 
				vendor3Address.Contains(carrierAddress) ? "Vendor3" : 
				"UNKNOWN",
	TotalEndpoints = acr.TotalEndpoints,
	NoResponse = acr.TotalEndpoints - acr.SuccessfulEndpoints
};

Generated SQL

DECLARE @__now_Date_0 datetime = '2021-10-11T00:00:00.000';

SELECT [o].[JobKey], [i].[StartTime], [o].[CellRelayId], [n].[SerialNumber], [r].[NativeAddress], [o].[MCHInstanceId], CASE
	WHEN CASE
		WHEN [r].[NativeAddress] IS NULL OR ([r].[NativeAddress] = '') THEN NULL
		ELSE SUBSTRING([r].[NativeAddress], 8 + 1, CAST(LEN([r].[NativeAddress]) AS int) - 8)
	END IS NULL OR ((CASE
		WHEN [r].[NativeAddress] IS NULL OR ([r].[NativeAddress] = '') THEN NULL
		ELSE SUBSTRING([r].[NativeAddress], 8 + 1, CAST(LEN([r].[NativeAddress]) AS int) - 8)
	END = '') AND CASE
		WHEN [r].[NativeAddress] IS NULL OR ([r].[NativeAddress] = '') THEN NULL
		ELSE SUBSTRING([r].[NativeAddress], 8 + 1, CAST(LEN([r].[NativeAddress]) AS int) - 8)
	END IS NOT NULL) THEN CAST(1 AS bit)
	ELSE CAST(0 AS bit)
END, CASE
	WHEN [r].[NativeAddress] IS NULL OR ([r].[NativeAddress] = '') THEN NULL
	ELSE SUBSTRING([r].[NativeAddress], 8 + 1, CAST(LEN([r].[NativeAddress]) AS int) - 8)
END, [o].[TotalEndpoints], [o].[TotalEndpoints] - [o].[SuccessfulEndpoints]
FROM [Analytics].[OwceCommRates] AS [o]
INNER JOIN [OWCE].[InterrogationJob] AS [i] ON CAST([o].[JobKey] AS numeric(9,0)) = [i].[JobKey]
INNER JOIN [OWCE].[RelayNode] AS [r] ON [o].[CellRelayId] = [r].[CellRelayId]
LEFT JOIN [OWCE].[Node] AS [n] ON [r].[NodeKey] = CAST([n].[RelayNodeKey] AS numeric(9,0))
WHERE ((((((([n].[NodeKey] IS NOT NULL 
AND (CONVERT(date, [i].[SubmittedTime]) > @__now_Date_0)) 
AND (DATEPART(hour, [i].[SubmittedTime]) < 12)) 
AND ([i].[JobStatus] = 'W')) 
AND (([o].[TotalEndpoints] - [o].[SuccessfulEndpoints]) > 0)) 
AND ([o].[CellRelayId] > 0)) AND [r].[SerialNumber] NOT IN (N'11', N'53098')) 
AND ([n].[HardwareVersion] IN (1.0, 3.0, 0.0) OR (CONVERT(int, [n].[SerialNumber]) > 9999999))) 
AND ([n].[Registered] = 1.0)

Versions

.net core 5.0
EF Core 5.0.8

@ajcvickers
Copy link
Contributor

/cc @smitpatel @maumar

@smitpatel
Copy link
Contributor

  1. This is just due to construction of expression tree by compiler into BinaryExpression. It is a binary tree so nesting will come which requires to put parenthesis to preserve operator precedence. The result should be same regardless.
  2. The lookup should translate to server, need to investigate why it isn't. A repro would be useful.

@sam-wheat
Copy link
Author

A repro would be useful.

Thanks @smitpatel, working on it.

@sam-wheat
Copy link
Author

Issue_26322.zip
Repro attached

@smitpatel
Copy link
Contributor

The argument of Contains is coming from result of "let" which makes it non-SqlExpression and we fail to translate it.

#20291 may be able to help. We should re-evaluate after that issue is fixed.

@ajcvickers ajcvickers added this to the Backlog milestone Oct 26, 2021
@smitpatel smitpatel removed their assignment Nov 19, 2021
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

3 participants