-
Notifications
You must be signed in to change notification settings - Fork 451
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
Add docs for SRF column accessors #3810
Conversation
Specific change is in the Table generator functions section. |
941128c
to
eee416f
Compare
@jseldess mind giving this one a look again? thanks! |
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.
thank you
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.
LGTM, with a few questions.
Reviewable status:
complete! 0 of 0 LGTMs obtained
v2.1/table-expressions.md, line 189 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql > SELECT * FROM generate_series(1, 3)
While you're here, would you mind adding a semicolon here so this can be pasted and run directly in the shell?
v2.1/table-expressions.md, line 203 at r2 (raw file):
<span class="version-tag">New in v2.1:</span> Set-returning functions (SRFs) can now be accessed using `(SRF).x` where `x` is either: - The name of a column returned from the function, or
nit: This isn't in our style guide, so it's up for debate, but I prefer to keep conjunctions out of bulleted lists. So I'd either introduce it a little differently (e.g., where x is either of the following:
) or just integrate into the sentence (e.g., where x is either the name of a column... or ...
).
v2.1/table-expressions.md, line 214 at r2 (raw file):
~~~ x | n
Should the above statement produce this output for me? I'm getting:
root@127.0.0.1:63810/defaultdb> SELECT (i.keys).* FROM (SELECT information_schema._pg_expandarray(indkey) AS keys FROM pg_index) AS i;
x | n
+---+---+
(0 rows)
Time: 2.935ms
Oh, also, is this PR fixing an issue? If so, can you use |
eee416f
to
9f55e1c
Compare
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.
Thanks @jseldess!
Reviewable status:
complete! 0 of 0 LGTMs obtained
v2.1/table-expressions.md, line 189 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
While you're here, would you mind adding a semicolon here so this can be pasted and run directly in the shell?
Fixed - thanks!
v2.1/table-expressions.md, line 203 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
nit: This isn't in our style guide, so it's up for debate, but I prefer to keep conjunctions out of bulleted lists. So I'd either introduce it a little differently (e.g.,
where x is either of the following:
) or just integrate into the sentence (e.g.,where x is either the name of a column... or ...
).
Fixed. Went with the "where x is one of the following: ..." formulation.
v2.1/table-expressions.md, line 214 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Should the above statement produce this output for me? I'm getting:
root@127.0.0.1:63810/defaultdb> SELECT (i.keys).* FROM (SELECT information_schema._pg_expandarray(indkey) AS keys FROM pg_index) AS i; x | n +---+---+ (0 rows) Time: 2.935ms
AFAICT the output varies depending on the contents of the current database. I think this is because information_schema
is used for name lookups by IDEs and things.
Fixed by updating to add "For example (note that the output of queries against information_schema
will vary per database):" to be (hopefully) clearer.
Fixes #3070