-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
PG: Support for multiple projection in aggregate #4469
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4469 +/- ##
==========================================
- Coverage 92.71% 92.71% -0.01%
==========================================
Files 118 118
Lines 8356 8355 -1
==========================================
- Hits 7747 7746 -1
Misses 609 609
Continue to review full report at Codecov.
|
@@ -1546,7 +1545,7 @@ export class PostgresStorageAdapter { | |||
} | |||
} | |||
|
|||
const qs = `SELECT ${columns} FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern} ${groupPattern}`; |
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 you sure you do not need to join the column there? Or is it the default for ES6 template strings when it comes to arrays?
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.
Did I reject this? Sorry, I'm not quite sure what I'm doing when it comes to reviews :)
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.
No problem. I did join the columns here. They weren't joined before properly.
@@ -1546,7 +1545,7 @@ export class PostgresStorageAdapter { | |||
} | |||
} | |||
|
|||
const qs = `SELECT ${columns} FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern} ${groupPattern}`; | |||
const qs = `SELECT ${columns.join()} FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern} ${groupPattern}`; |
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.
@vitaly-t Joined 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.
LGTM.
Adds support for multiple keys in aggregate queries.
Improves Projection Tests
Lints PG adapter.