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

sql docs: add detail to documentation for transaction_timestamp #20075

Closed
couchand opened this issue Nov 15, 2017 · 3 comments
Closed

sql docs: add detail to documentation for transaction_timestamp #20075

couchand opened this issue Nov 15, 2017 · 3 comments

Comments

@couchand
Copy link
Collaborator

@couchand couchand commented Nov 15, 2017

The documentation for the built-inscurrent_timestamp(), now(), and transaction_timestamp() says simply "Returns the current transaction’s timestamp." This is a source of confusion, since the phrase "transaction’s timestamp" seems to indicate that it is the same value as described in the transaction docs and the architecture docs ("Whenever a transaction's timestamp is mentioned, it's an HLC value."). It's not, but we don't make that very clear.

We should expand that documentation to include more detail around exactly what timestamp is returned from these built-ins, and how that differs from the internal transaction timestamp.

@couchand

This comment has been minimized.

Copy link
Collaborator Author

@couchand couchand commented Nov 15, 2017

Assigning to @awoods187 for triage.

@bdarnell

This comment has been minimized.

Copy link
Member

@bdarnell bdarnell commented Nov 15, 2017

I was also tripped up by this - I thought that transaction_timestamp() was just cluster_logical_timestamp() truncated to sql timestamp resolution. I don't remember why we made this decision and I'm sure there was a good reason, but it would be great if these two could be made consistent instead of just documenting the difference between them.

@couchand

This comment has been minimized.

Copy link
Collaborator Author

@couchand couchand commented Nov 15, 2017

@ben for context, here's the issue where the change was made: #5767

@knz knz added docs-todo and removed O-community-questions labels Apr 24, 2018
@knz knz added this to Backlog in (DEPRECATED) SQL Front-end, Lang & Semantics via automation Apr 24, 2018
@knz knz moved this from Triage to Current milestone in (DEPRECATED) SQL Front-end, Lang & Semantics May 4, 2018
craig bot pushed a commit that referenced this issue May 9, 2018
25327: sql: improve the documentation of the timestamp functions r=knz a=knz

Fixes #20075.
Fixes #20076.

Release note (sql change): the online documentation for `now()`,
`current_time()`, `current_date()`, `current_timestamp()`,
`clock_timestamp()`, `statement_timestamp()` and
`cluster_logical_timestamp()` has been improved and clarified.



Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig craig bot closed this in #25327 May 9, 2018
@knz knz moved this from Current milestone to Finished (milestone 0423) in (DEPRECATED) SQL Front-end, Lang & Semantics May 9, 2018
@knz knz removed the docs-todo label May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.