-
Notifications
You must be signed in to change notification settings - Fork 9
DARYNA-TKACHENKO-W2-DATABASE #18
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?
DARYNA-TKACHENKO-W2-DATABASE #18
Conversation
crevulus
left a comment
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.
Hi Daryna,
There's an issue I've raised in my comments - there's important data missing, so I can't review your code fully! Please fix this, along with the other comments, and then I can leave a full review.
| @@ -0,0 +1,75 @@ | |||
| import { client } from './database.js'; | |||
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 file name contains a trailing whitespace. This can cause issues for running your scripts. Be careful of whitespace when naming.
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.
@dashaaaa21 This is still unresolved
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.
Week2/assigments/relationships.js
Outdated
| INSERT INTO authors (author_name) | ||
| VALUES | ||
| ('Daryna Tkachenko'), | ||
| ('Andrii Shevchenko'), |
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.
Great choice of names! ⚽
However, you're missing quite a lot of data from this table. I'm unable to verify exercises 3 and 4 because of missing data from the authors table.
| `); | ||
|
|
||
|
|
||
| await client.query(` |
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.
here you're doubling-up your work. You have already created the table as part of an earlier exercise. No need to do it again.
|
|
||
| await client.query(` | ||
| CREATE TABLE author_paper ( | ||
| author_id INT, |
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.
well done for achieving this in one query instead of two 👍
Week2/assigments/relationships.js
Outdated
| (13, 25), (13, 26), | ||
| (14, 27), (14, 28), | ||
| (15, 29), (15, 30) | ||
| ON CONFLICT DO NOTHING; |
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 good habit, but it's not strictly necessary because there's no UNIQUE constraint on author_paper
Week2/assigments/joins.js
Outdated
| console.table(mentorResult.rows); | ||
|
|
||
| const papersQuery = ` | ||
| SELECT a.author_id, |
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.
There's an easier way to select all columns without having to write out each one. Can you remember what it is?
|
|
||
| const papersCountQuery = ` | ||
| SELECT rp.paper_title, | ||
| COUNT(ap.author_id) AS author_count |
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 runs, but you might want to be more specific and declare where you want to select the paper titles and author ids from. Otherwise, postgres will implicitly assume which table to use. Which in this case is okay, but in a real production database may cause issues.
No description provided.