-
-
Notifications
You must be signed in to change notification settings - Fork 14
Add basic SELECT concept/exercise #223
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
base: main
Are you sure you want to change the base?
Add basic SELECT concept/exercise #223
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.
Are hints needed for this one? I'm open to suggestions...
| -- TODO: Add the appropriate SELECT statement here | ||
| ; |
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.
If the user runs the tests on this stub file, it will result in syntax errors. There would also be syntax errors if any of the CREATE TABLE stubs are left unmodified, which would make it unlikely for the user to solve each task individually, in TDD fashion. Maybe it would be better to include a SELECT stub in each, like
| -- TODO: Add the appropriate SELECT statement here | |
| ; | |
| -- TODO: Update the SELECT statement as needed here | |
| SELECT "TODO" | |
| ; |
Other thoughts?
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.
Hard agree. The stub should be valid SQL. Maybe drop the TODO comment and just do something like SELECT "Replace this SELECT statement with the correct SELECT statement to solve this task"
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.
|
|
||
| ## The basics | ||
|
|
||
| The anatomy of a basic `SELECT` statement is as follows: |
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.
| The anatomy of a basic `SELECT` statement is as follows: | |
| The anatomy of a basic `SELECT` statement is as follows. |
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.
Why not :?
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.
This is a complete sentence and complete idea as is.
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 agree it's a complete sentence, but I don't agree that it's a complete idea. I think the information following "as follows" is essential to understanding the sentence, hence the colon. Anyway, if there is an existing convention to use a period after "as follows," I will follow it. Otherwise, I would prefer to use a colon.
| .mode markdown | ||
| .output user_output.md | ||
| .read ./intro-select.sql | ||
| .shell rm -f ./results.db |
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.
Is this line needed if the save overrides?
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.
Not necessary, but I thought it would be nice to leave the workspace clean.
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.
The immediate next step writes this file, though. How does this result in a cleaner workspace?
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.
D'oh!
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.
Moved the removal to the end of the file.
| .output | ||
|
|
||
| -- Report results | ||
| .shell bash ./report-results.sh results.db |
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.
Thoughts on dropping the sh extension? This isn't a sh script.
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.
Sure. Should it be .bash, or no extension?
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.
Personally I'd go with no extension.
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.
| else | ||
| (got | rows | tostring) as $got_rows | ||
| | (expected | rows | tostring) as $expected_rows | ||
| | "With columns \($got_columns)\nexpected \($expected_rows)\nbut got \($got_rows)" |
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.
| | "With columns \($got_columns)\nexpected \($expected_rows)\nbut got \($got_rows)" | |
| | "With columns \($got_columns)\nexpected row \($expected_rows)\nbut got \($got_rows)" |
To match the column error
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.
Incorporates ideas from #217, #218 and #219.