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

Set Size to -1 to optimize SQL Server query plan execution #1254

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

ramonsmits
Copy link
Member

@ramonsmits ramonsmits commented Jul 31, 2023

Size needs to be set to -1

Smoketested using the SQL Outbox sample:

  • Cloned Core to Core v9 and target locally created nugets from this branch.
  • Modified the sample to have a text property increase in length for each message send
  • Ran the sample where a message and sent a bunch of messages
  • Cleared the query cache DBCC FREEPROCCACHE
  • Ran the following query to observe the query cache statistics:
     SELECT
     	databases.name,
     	dm_exec_sql_text.text AS TSQL_Text,
     	dm_exec_query_stats.execution_count
     FROM sys.dm_exec_query_stats 
     CROSS APPLY sys.dm_exec_sql_text(dm_exec_query_stats.plan_handle)
     INNER JOIN sys.databases
     ON dm_exec_sql_text.dbid = databases.database_id
     WHERE databases.name<>'master'
     ORDER BY TSQL_Text
  • Observed in the result that it works:
     (@MessageId nvarchar(4000),@Operations nvarchar(4000),@PersistenceVersion nvarchar(4000))  insert into [sender].[OutboxData]  (      MessageId,      Operations,      PersistenceVersion  )  values  (      @MessageId,      @Operations,      @PersistenceVersion  )
     (@MessageId nvarchar(4000),@Operations nvarchar(4000),@PersistenceVersion nvarchar(4000))  insert into [sender].[OutboxData]  (      MessageId,      Operations,      PersistenceVersion  )  values  (      @MessageId,      @Operations,      @PersistenceVersion  )
     (@MessageId nvarchar(4000),@Operations nvarchar(max) ,@PersistenceVersion nvarchar(4000))  insert into [receiver].[OutboxData]  (      MessageId,      Operations,      PersistenceVersion  )  values  (      @MessageId,      @Operations,      @PersistenceVersion  )
     (@MessageId nvarchar(4000),@Operations nvarchar(max) ,@PersistenceVersion nvarchar(4000))  insert into [receiver].[OutboxData]  (      MessageId,      Operations,      PersistenceVersion  )  values  (      @MessageId,      @Operations,      @PersistenceVersion  )

@ramonsmits ramonsmits self-assigned this Jul 31, 2023
@ramonsmits ramonsmits added this to the vNext milestone Aug 1, 2023
@github-actions github-actions bot added the stale label Sep 1, 2023
@ramonsmits ramonsmits marked this pull request as ready for review September 4, 2023 23:22
@Particular Particular deleted a comment from github-actions bot Sep 4, 2023
@github-actions github-actions bot removed the stale label Sep 5, 2023
@github-actions github-actions bot added the stale label Oct 5, 2023
@ramonsmits ramonsmits removed the stale label Oct 9, 2023
@Particular Particular deleted a comment from github-actions bot Oct 9, 2023
@mauroservienti
Copy link
Member

@ramonsmits are you taking care of the smoke testing bit?

@ramonsmits
Copy link
Member Author

@mauroservienti done! As this is an enhancement what do you think? Only release this for the next major or should this be backported to the current release major?

@mauroservienti
Copy link
Member

I think it should be backported to the currently released major. That's the only release available to users, the next major is in the works but there is no ETA.

Copy link
Member

@tmasternak tmasternak left a comment

Choose a reason for hiding this comment

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

Nice improvement. Could we make the parameters lengths explicit?

@ramonsmits
Copy link
Member Author

Although these last commits use the actual lengths of the schema it feels error-prone. It would require additional refactoring to really make the lengths part of a central schema to get the length information which is a major code change.

Dapper uses -1 as the default is a string unless the actually string length is less than 4000 and then uses 4000 UNLESS a explicit length was configured

I vote for reverting these changes revert to e0d21e0. That behavior is pretty much as dapper except that dapper sets length to -1 when its a null value. That logic doesn't make sense IMHO and we can keep the behavior at e0d21e0.

@tmasternak @mauroservienti what do you think?

@mauroservienti
Copy link
Member

Agreed. 👍 to revert to e0d21e0

This reverts commit 8ff785a.

Revert "Set default lengths as suggested by @tmasternak"

This reverts commit f668d68.
@ramonsmits ramonsmits merged commit 2303481 into master Oct 24, 2023
9 checks passed
@ramonsmits ramonsmits deleted the sqlparameter-size branch October 24, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Higher SQL Server CPU/RAM utilization SQL dialect queries not hitting SQL query cache
4 participants