From 4272e755b944c8554d5c12fe7b93cbeac24c23f2 Mon Sep 17 00:00:00 2001 From: Kulshekhar Kabra Date: Fri, 25 Nov 2016 00:09:23 +0530 Subject: [PATCH 1/4] Proposal for 'distinct' query --- proposals/0002-query-distinct.md | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 proposals/0002-query-distinct.md diff --git a/proposals/0002-query-distinct.md b/proposals/0002-query-distinct.md new file mode 100644 index 0000000..7190446 --- /dev/null +++ b/proposals/0002-query-distinct.md @@ -0,0 +1,35 @@ +# Feature name + +* Proposal: 0002-query-distinct +* Authors: [Kulshekhar Kabra](https://github.com/kulshekhar) +* Review Manager: TBD +* Status: **Awaiting review** + +## Introduction + +Allow users to fetch distinct values for a column that matches a given query + +## Motivation + +It is an often requested and useful feature. + +Related issue: ParsePlatform/parse-server#2238 + +## Proposed solution + +Introducing an additional parameter `distinct` (similar to the existing `keys`, `order`, `limit`, `skip` and `include` parameters) + +## Detailed design + +The new parameter will need to be whitelisted in `ClassesRouter.js`. It will also need to go through any sanitization processes that exist to prevent injection attacks. + +Additionally, the presence of this parameter should modify the queries as follows: + +- In MongoDB, execute the `.distinct()` method instead of the usual `find` query +- In Postgres, use `DISTINCT(fieldname)` instead of the usual list of field names in the `find` query + +## Alternatives considered + +Another alternative that could work would be to allow modifiers with field names in the `keys` parameter. Something like `distinct:field1,field2`. + +However, this seems brittle and likely would introduce a point for SQL injection/other issues. From 7c1fc9ac33e07b824c2878d337b462f9f699c0cb Mon Sep 17 00:00:00 2001 From: Kulshekhar Kabra Date: Fri, 25 Nov 2016 00:12:34 +0530 Subject: [PATCH 2/4] Link to existing issue --- proposals/0002-query-distinct.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/0002-query-distinct.md b/proposals/0002-query-distinct.md index 7190446..b997969 100644 --- a/proposals/0002-query-distinct.md +++ b/proposals/0002-query-distinct.md @@ -13,7 +13,7 @@ Allow users to fetch distinct values for a column that matches a given query It is an often requested and useful feature. -Related issue: ParsePlatform/parse-server#2238 +[Related issue](https://github.com/ParsePlatform/parse-server/issues/2238) ## Proposed solution From 227cd19177fbd3e99d652b5f4c3eda4b00020dc1 Mon Sep 17 00:00:00 2001 From: Kulshekhar Kabra Date: Fri, 25 Nov 2016 00:19:42 +0530 Subject: [PATCH 3/4] Note how keys/include parameters will be handled --- proposals/0002-query-distinct.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/proposals/0002-query-distinct.md b/proposals/0002-query-distinct.md index b997969..0bceb63 100644 --- a/proposals/0002-query-distinct.md +++ b/proposals/0002-query-distinct.md @@ -28,6 +28,8 @@ Additionally, the presence of this parameter should modify the queries as follow - In MongoDB, execute the `.distinct()` method instead of the usual `find` query - In Postgres, use `DISTINCT(fieldname)` instead of the usual list of field names in the `find` query +If the `distinct` parameter is present in addition to the `keys` and/or `include` parameters, I'd argue that these (`keys`/`include`) should be ignored as they wouldn't really make sense. Any documentation for `distinct` should probably include a mention of this decision. + ## Alternatives considered Another alternative that could work would be to allow modifiers with field names in the `keys` parameter. Something like `distinct:field1,field2`. From 9dadd36713de54772578f76f20105694d3b64523 Mon Sep 17 00:00:00 2001 From: Kulshekhar Kabra Date: Fri, 25 Nov 2016 02:49:38 +0530 Subject: [PATCH 4/4] Expand the scope of the proposal --- proposals/0002-query-distinct.md | 83 ++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/proposals/0002-query-distinct.md b/proposals/0002-query-distinct.md index 0bceb63..de6d6cb 100644 --- a/proposals/0002-query-distinct.md +++ b/proposals/0002-query-distinct.md @@ -1,37 +1,88 @@ # Feature name -* Proposal: 0002-query-distinct +* Proposal: 0002-query-aggregate * Authors: [Kulshekhar Kabra](https://github.com/kulshekhar) * Review Manager: TBD * Status: **Awaiting review** ## Introduction -Allow users to fetch distinct values for a column that matches a given query +Allow users to make queries that use aggregates (distinct, sum, count, etc) ## Motivation -It is an often requested and useful feature. +It is an often requested and useful feature which would enhance the querying capability offered by parse server -[Related issue](https://github.com/ParsePlatform/parse-server/issues/2238) +One [Related issue](https://github.com/ParsePlatform/parse-server/issues/2238) ## Proposed solution -Introducing an additional parameter `distinct` (similar to the existing `keys`, `order`, `limit`, `skip` and `include` parameters) +Introduce two additional parameters (similar to the existing `keys`, `order`, `limit`, `skip` and `include` parameters) -## Detailed design - -The new parameter will need to be whitelisted in `ClassesRouter.js`. It will also need to go through any sanitization processes that exist to prevent injection attacks. - -Additionally, the presence of this parameter should modify the queries as follows: +- `group`: this will identify the fields and the aggregate functions to be applied to them, if any +- `having`: this would be similar to the `where` clause but would be applicable to aggregated fields. -- In MongoDB, execute the `.distinct()` method instead of the usual `find` query -- In Postgres, use `DISTINCT(fieldname)` instead of the usual list of field names in the `find` query +## Detailed design -If the `distinct` parameter is present in addition to the `keys` and/or `include` parameters, I'd argue that these (`keys`/`include`) should be ignored as they wouldn't really make sense. Any documentation for `distinct` should probably include a mention of this decision. +The new parameters will need to be whitelisted in `ClassesRouter.js`. They will also need to go through any sanitization processes that exist to prevent injection attacks. + +The `having` parameter without the `group` parameter would be ignored. + +The specification for the `group` parameter can be based on the `$group` operator from MongoDB. The operators allowed can differ based on the database adapter (The next section contains a list of operators available in Postgres and MongoDB) + +The specification for the `having` parameter can be based on the existing `where` parameter. + +The adapters will need to modify their query generation functions to incorporate these changes + +There is a direct relation between the new parameters introduced and Postgres's query semantics. + +For MongoDB the `group` parameter has a direct implementation in `$group`. To implement `having`, inspiration can be taken from [this PDF](https://rickosborne.org/download/SQL-to-MongoDB.pdf) + +If the `group` parameter is present in addition to the `keys` and/or `include` parameters, I'd argue that these (`keys`/`include`) should be ignored as they wouldn't really make sense. Any documentation for `group` should probably include a mention of this decision. + +## Aggregate operators + +|Postgres|MongoDB| +|---|---| +||$addToSet| +|array_agg|$push| +|avg|$avg| +|bit_and| +|bit_or| +|bool_and| +|bool_or| +|count| +|every| +||$first +|json_agg|$push +|jsonb_agg|$push +|json_object_agg| +|jsonb_object_agg| +||$last +|max|$max +|min|$min +|string_agg| +|sum|$sum +|xmlagg| +|corr| +|covar_pop| +|covar_samp| +|regr_avgx| +|regr_avgy| +|regr_count| +|regr_intercept| +|regr_r2| +|regr_slope| +|regr_sxx| +|regr_sxy| +|regr_syy| +|stddev| +|stddev_pop|$stdDevPop +|stddev_samp|$stdDevSamp +|variance| +|var_pop| +|var_samp| ## Alternatives considered -Another alternative that could work would be to allow modifiers with field names in the `keys` parameter. Something like `distinct:field1,field2`. - -However, this seems brittle and likely would introduce a point for SQL injection/other issues. +None really