-
Notifications
You must be signed in to change notification settings - Fork 64
Update README.md #627
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
Update README.md #627
Conversation
Add note about the current datetime format behavior difference between C# in-process and other languages.
@@ -54,7 +55,6 @@ Databases on SQL Server, Azure SQL Database, or Azure SQL Managed Instance which | |||
- PowerShell Functions using Output bindings cannot pass in null or empty values via the query string. The workaround is to use the `$TriggerMetadata[$keyName]` to retrieve the query property, an example can be found [here](https://github.com/Azure/azure-functions-sql-extension/blob/main/samples/samples-powershell/AddProductParams/run.ps1). Issue is tracked [here](https://github.com/Azure/azure-functions-powershell-worker/issues/895). | |||
|
|||
### Input Bindings | |||
- Input bindings against tables with columns of data types 'DATETIME', 'DATETIME2', or 'SMALLDATETIME' will assume that the values are in UTC format. |
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.
Removing this since Datetime types dont have time zone information. SQL DatetimeOffset, which includes the time zone, gets converted to C# DatetimeOffset type correctly.
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.
But when we convert the types don't we end up with time values that are in UTC (because they don't have a timezone offset to go off of)? This is just telling the user that if they have those types we can't tell what the timezone is so it assumes UTC.
Maybe we should just add another line explaining that if the timestamp being stored isn't UTC they should use datetimeoffset instead?
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.
Hm I might be misunderstanding something here but since the CSharp Datetime type doesn't have a time zone field, when we store a DATETIME value from the SQL table in a CSharp Datetime object, it technically isn't in either UTC or local format. We are just storing the datetime in the object as is and not doing any time zone conversions (ex. we're not taking a local datetime and converting it to UTC).
I think using DATETIME vs DATETIMEOFFSET is up to the user when creating their database and not necessarily an issue we should note with sql bindings.
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.
Ok, I think that makes sense. One of these days I need to actually spend time playing around with this myself though...
Co-authored-by: Charles Gagnon <[email protected]>
@@ -54,7 +55,6 @@ Databases on SQL Server, Azure SQL Database, or Azure SQL Managed Instance which | |||
- PowerShell Functions using Output bindings cannot pass in null or empty values via the query string. The workaround is to use the `$TriggerMetadata[$keyName]` to retrieve the query property, an example can be found [here](https://github.com/Azure/azure-functions-sql-extension/blob/main/samples/samples-powershell/AddProductParams/run.ps1). Issue is tracked [here](https://github.com/Azure/azure-functions-powershell-worker/issues/895). | |||
|
|||
### Input Bindings | |||
- Input bindings against tables with columns of data types 'DATETIME', 'DATETIME2', or 'SMALLDATETIME' will assume that the values are in UTC format. |
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.
Ok, I think that makes sense. One of these days I need to actually spend time playing around with this myself though...
* Update README.md Add note about the current datetime format behavior difference between C# in-process and other languages. * move to output binding section * Update README.md Co-authored-by: Charles Gagnon <[email protected]> Co-authored-by: Charles Gagnon <[email protected]>
Add note about the current datetime format behavior difference between C# in-process and other languages.