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

Remove References to AT TIME ZONE in FOR SYSTEM_TIME AS OF #9758

Closed
wants to merge 1 commit into from

Conversation

LarnuUK
Copy link
Contributor

@LarnuUK LarnuUK commented Apr 16, 2024

The documentation states that AT TIME ZONE can be used in conjunction with FOR SYSTEM_TIME AS FOR. The example gives, however, gives an error:

Incorrect syntax near 'AT'.

This is true if you use the clause against a VIEW or a TABLE, the same error occurs. db<>fiddle

It appears, therefore, that the documentation is incorrect, and the section should be removed, which this PR does.

If wanted, however, I am happy to add an example that uses a variable of datetimeoffset with AT TIME ZONE used against it, but I would want to check out how this works before doing so, to make sure the documentation is informative.

The documentation states that `AT TIME ZONE` can be used in conjunction with `FOR SYSTEM_TIME AS FOR`. The example gives, however, gives an error:
> Incorrect syntax near 'AT'.

This is true if you use the clause against a `VIEW` or a `TABLE`, the same error occurs. [db<>fiddle](https://dbfiddle.uk/6Kv6-9o6)

It appears, therefore, that the documentation is incorrect, and the section should be removed, which this PR does.

*If wanted, however, I am happy to add an example that uses a variable of `datetimeoffset` with `AT TIME ZONE` used against it, but I would want to check out how this works before doing so the documentation is informative.*
Copy link
Contributor

@LarnuUK : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit d90a4c1:

✅ Validation status: passed

File Status Preview URL Details
docs/relational-databases/tables/querying-data-in-a-system-versioned-temporal-table.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@Court72
Copy link
Contributor

Court72 commented Apr 16, 2024

@rwestMSFT

Can you review the proposed changes?

When the changes are ready for publication, add a #sign-off comment to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged tracking label for the PR review team label Apr 16, 2024
@rwestMSFT
Copy link
Contributor

Hi @LarnuUK

I originally authorized the PR last year that added this change, and didn't do due diligence to test it properly beforehand.

On the WideWorldImporters database, you can do this:

DECLARE @CustomerTimeZone NVARCHAR(128) = 'Central European Standard Time';

SELECT StockItemID, ValidFrom,
    --Assign the known offset only
    ValidFrom AT TIME ZONE 'Pacific Standard Time' AS OrderDate_TimeZonePST,
    --Assign the known offset, then convert to another time zone
    ValidFrom AT TIME ZONE @CustomerTimeZone AS OrderDate_TimeZoneCustomer
FROM Warehouse.StockItems;

However, you are correct, and AT TIME ZONE is not compatible with AS OF.

Instead of approving your PR directly, which removes the example entirely, I'm going to roll back the change from last year, but you will still get contributor credit.

Thanks for your contribution.

@LarnuUK
Copy link
Contributor Author

LarnuUK commented Apr 16, 2024

Thanks @rwestMSFT . I'll close this PR then as well, as a roll back does solve the problem as well.

@LarnuUK LarnuUK closed this Apr 16, 2024
@LarnuUK LarnuUK deleted the patch-8 branch April 17, 2024 16:04
@rwestMSFT
Copy link
Contributor

@LarnuUK Thanks - I'll comment here once I've refreshed all the articles in this area, because some of them haven't been looked at in a while.

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