-
Notifications
You must be signed in to change notification settings - Fork 121
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 SQL style guide #133
Add SQL style guide #133
Conversation
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 for this, it's a great start!
One objection to a rule and a couple of nits.
concepts/sql_style.md
Outdated
## Reserved Words | ||
|
||
1. Always use uppercase for reserved keywords like `SELECT`, `WHERE`, or `AS`. | ||
1. Always use uppercase for Presto functions, such as `COUNT`, `CARDINALITY`, or `SUBSTR`. |
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.
I figured out why I dislike uppercase function names - function names are identifiers, so should be treated like tables, fields, or variables (rather than like reserved keywords).
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.
Agreed. Fixed.
|
||
1. Use consistent and descriptive identifiers and names. | ||
1. Use lower case names with underscores, such as `first_name`. | ||
Do not use CamelCase. |
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.
nit: surround CamelCase with backticks
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.
I've added this to .spelling instead.
concepts/sql_style.md
Outdated
|
||
1. The opening parenthesis should terminate the line. | ||
1. The closing parenthesis should be lined up under | ||
the first character of the line that starts the multiline construct. |
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.
sp: "multi-line"
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.
Done
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.
Splendid! 🥇
No description provided.