-
Notifications
You must be signed in to change notification settings - Fork 294
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
FIX | db_id returns null on Azure SQL Database. #1294
Conversation
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs
Outdated
Show resolved
Hide resolved
} | ||
reader.Close(); | ||
|
||
if (dbId is null || !dbId.Value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add test for code that currently throws NRE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be shorter as !dbid.GetValueOrDefault()
, I'm not sure if it's clearer and perf isn't a major concern just a general one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the test, it needs more investigation as this only happen on Azure SQL DB and even in this case it may return null.
GetValueOrDefault returns a value even if the HasValue property is false. This also needs more cautious investigation performance wise and also returned value as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetValueOrDefault returns a value even if the HasValue property is false
Yup, and it does so in a single call with a smaller number of instructions than doing the null check then value check. As long as you have that leading negation it would work. But as I said it's hardly a performance problem so leave it as it already is.
Copying this over from #1264: The issue is not that db_id() returns null - it doesn't. The issue is that, as documented, its value is nonsensical on Azure SQL. |
Please, resolve the conflict. |
closes Issue #1264.
db_id()
returns null on Azure SQL Database. The proposed fix adds a nullable boolean and assign the values from the reader to it.Also
ExecuteScalar
is replaced withExecuteReader(CommandBehavior.SingleRow)
.The reason to do so was avoiding the cast on the result of returned object from
ExecuteScalar
and since db_id() returns the database ID of the current database we only need one row.