Skip to content

Conversation

@morgo
Copy link
Contributor

@morgo morgo commented Oct 8, 2018

Incorrectly stated only-full-group-by was not supported.

Note: This PR references aggregate functions that are in development and have not been merged yet. If they are not complete after review, I will move them to the unsupported functions section.

Incorrectly stated only-full-group-by was not supported.
@morgo morgo requested review from CaitinChen and zz-jason October 8, 2018 21:07
@morgo morgo changed the title sql: Improved aggregation docs sql: Improve aggregation docs Oct 8, 2018
| [`STDDEV_SAMP()`](https://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html#function_stddev-samp) | Return the sample standard deviation |
| [`VARIANCE()`](https://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html#function_variance) | Return the population standard variance |
| [`VAR_POP()`](https://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html#function_var-pop) | Return the population standard variance |
| [`JSON_ARRAYAGG()`](https://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html#function_json-arrayagg) | Return result set as a single JSON array |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these functions are not supported yet, their PRs are in review.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment in the description- I will move them to ‘unsupported’ if they do not merge before review of this PR is complete

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the PR to only include merged functions. PTAL @zz-jason

## GROUP BY modifiers

TiDB dose not support any `GROUP BY` modifiers currently. We'll do it in the future. For more information, see [#4250](https://github.com/pingcap/tidb/issues/4250).
TiDB does not currently support `GROUP BY` modifiers such as `WITH ROLLUP`. We plan to add support in the future. See [TIDB #4250](https://github.com/pingcap/tidb/issues/4250).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIDB #4250 -> TiDB #4250

TiDB performs equivalent to MySQL with sql mode [`ONLY_FULL_GROUP_BY`](https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_only_full_group_by) being disabled: permits the `SELECT` list, `HAVING` condition, or `ORDER BY` list to refer to non-aggregated columns even if the columns are not functionally dependent on `GROUP BY` columns.

For example, this query is illegal in MySQL 5.7.5 with `ONLY_FULL_GROUP_BY` enabled because the non-aggregated column "b" in the `SELECT` list does not appear in the `GROUP BY`:
TiDB supports the SQL Mode `ONLY_FULL_GROUP_BY`, and when enabled TiDB will refuse queries with ambiguous non-aggregated columns. For example, this query is illegal with `ONLY_FULL_GROUP_BY` enabled because the non-aggregated column "b" in the `SELECT` list does not appear in the `GROUP BY`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the GROUP BY -> the GROUP BY statement
OR:
the GROUP BY -> the GROUP BY

```

The preceding query is legal in TiDB. TiDB does not support SQL mode `ONLY_FULL_GROUP_BY` currently. We'll do it in the future. For more inmormation, see [#4248](https://github.com/pingcap/tidb/issues/4248).
TiDB does not currently enable [`ONLY_FULL_GROUP_BY`](mysql-compatibility.md#default-differences) mode by default.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enable -> enable the ... mode


### Differences from MySQL

The current implementation of `ONLY_FULL_GROUP_BY` is less strict than in MySQL 5.7. For example, suppose that we execute the following query, expecting the results to be ordered by "c":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

than in MySQL -> than that in MySQL

## Detection of functional dependence
## Unsupported aggregate functions

The following aggregate functions are currently unsupported in TiDB. You can track our progress in [TIDB #7623](https://github.com/pingcap/tidb/issues/7623):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Remove one extra space before "You can". Only leave one.
  • TIDB 7623 -> TiDB 7623

@lilin90 lilin90 changed the title sql: Improve aggregation docs sql: improve aggregation docs Oct 11, 2018
Copy link
Member

@lilin90 lilin90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morgo
Copy link
Contributor Author

morgo commented Oct 14, 2018

I've removed the yet-to-be-merged functions, so it is easier to review.
PTAL @zz-jason

@morgo morgo mentioned this pull request Oct 16, 2018
11 tasks
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morgo morgo merged commit 9a4fac4 into master Oct 19, 2018
@morgo morgo deleted the morgo/only-full-group-by branch October 19, 2018 04:32
lilin90 added a commit to lilin90/docs-cn that referenced this pull request Oct 26, 2018
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution This PR is from a community contributor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants