Fixed the large files issue in PR#658#660
Open
hlosukwakha wants to merge 7 commits into
Open
Conversation
… wildcard expansion)
```markdown # Title **tests(select_exclude) + sql(select_exclude): implement `SELECT ... EXCLUDE(...)` and normalize mysql-test files** --- ## Summary Implements the SQL extension `SELECT ... EXCLUDE(...)` end-to-end and normalizes the mysql-test files required to validate the feature. - Adds `EXCLUDE` grammar and parser support. - Attaches exclude-list metadata to the AST `Item_asterisk`. - Propagates exclude-lists through the resolver into wildcard expansion. - Normalizes/cleans MTR test files so the local harness passes (removes code fences, fixes spacing/newlines, aligns `.result` output with harness logs). - This MR contains both the source changes and the corresponding test-suite updates necessary to validate the feature locally. --- ## What changed (high level) ### Source - **Parser** - `sql_yacc.yy` — new `EXCLUDE` production to parse `EXCLUDE (ident_list)` and build parser-allocated lists. - **AST / Items** - `item.h` — `Item_asterisk` extended to hold an exclude-list pointer. - **Wildcard expansion / helpers** - `sql_base.h` / `sql_base.cc` — `insert_fields` updated to accept and apply exclude lists when expanding `*`. - **Resolver** - `sql_resolver.cc` — `Query_block::setup_wild` reads `Item_asterisk::m_exclude_list` and forwards it to the expansion routine. - Minor supporting changes to show/processlist files (to avoid accidental regression while adding the feature). ### Tests - `select_exclude_multi_table.test` and `.result` — multi-table tests for `EXCLUDE(...)` (cleaned of formatting artifacts and normalized). - `select_exclude_edgecases.test` and `.result` — edge-case tests (duplicates, excluding all columns, nested selects); cleaned and normalized. - (See full file list below.) --- ## Files changed (representative) - `sql_yacc.yy` - `item.h` - `sql_base.h` - `sql_base.cc` - `sql_resolver.cc` - `show_query_builder.cc` - `sql_show_processlist.cc` - `sql_show_status.cc` - `select_exclude_multi_table.test` - `select_exclude_multi_table.result` - `select_exclude_edgecases.test` - `select_exclude_edgecases.result` --- ## Commits - `f1d5595f92b` — **tests(select_exclude):** normalize test and `.result` files for select_exclude feature - `e7d7709dc4c` — **sql(select_exclude):** implement `EXCLUDE` syntax (parser, `Item_asterisk`, wildcard expansion) --- ## Why this change The `EXCLUDE(...)` syntax provides a convenient way to select all columns except a small list of columns (PII, secret fields, etc.) without enumerating the full column list. - Adds end-to-end support (parse → AST → resolve → expand) and tests to validate behavior. --- ## Backwards compatibility / risk - This introduces a new SQL grammar construct. It is additive and does not change existing syntax. - Minimal risk to existing features if the exclude list is simply ignored for contexts that don't use it. - Tests were normalized, and I used the exact harness output to ensure MTR compatibility — please double-check CI environments for any locale/ICU differences. --- ## How to test locally (quick) From the repository root: 1. Build/run your usual out-of-source CMake build as you normally would (if you need to run a full server). 2. Run just the affected MTR tests (fast local check). Expected: the three tests pass locally (they passed for me after normalizing the tests). Inspect logs if needed: - `mysql-test/var/log/local.select_exclude_multi_table/` - `mysql-test/var/log/local.select_exclude_edgecases/` --- ## CI considerations The test normalization was done to match the local harness. CI may run in a slightly different environment (ICU/data directories, locale, or tab/space rendering). If CI shows mismatches: - Confirm the CI runtime and mysql-test environment (ICU data paths or locale). - If message wording differs on CI, update `.result` to the canonical CI output or make the test less strict where appropriate. - Consider adding the new local tests to the CI collection that runs for feature branches. --- ## Notes / implementation details - **Parser:** new `opt_exclude` production returns a `List<String> *` allocated by the parser; this is attached to `Item_asterisk`. - **AST:** `Item_asterisk` stores `m_exclude_list` and forwards it during resolve. - `insert_fields(...)` now accepts an exclude-list pointer and filters column names before appending them to the select list. - **Resolver:** `Query_block::setup_wild` detects `Item_asterisk` and forwards the exclude list to `insert_fields`. --- ## Review checklist - Parser changes: scanning and bison-generated output reviewed for correctness. - `Item_asterisk` changes: ensure memory ownership expectations are respected when parser-allocated lists are attached to AST nodes. - `insert_fields` changes: test with qualified/unqualified stars, duplicate names, and multi-table joins. - Run impacted unit tests and MTR suite subset. - Confirm there are no unexpected changes to `mysqld` error or logging wording. --- ## Suggested reviewers - SQL parser / grammar maintainers - SQL resolver / query-expansion owners - MTR/test-maintainers for the mysql-test changes --- ## Additional follow-ups (optional) Expand MTR coverage to cover: - Qualified vs unqualified stars in complex join/CTE scenarios. - Interaction with `SELECT ... INTO` and `INSERT ... SELECT`. - Behavior when exclusion lists include columns from multiple source tables with the same name. Other: - Add inline comments to new parser/AST fields to describe memory ownership and lifetime expectations. - Consider adding integration tests that run under CI's environment to avoid environment-specific `.result` mismatches (locale/ICU). --- ```
## New Feature: `GROUP BY ALL` Support ### Description MySQL now supports the `GROUP BY ALL` clause. It automatically groups by all non-aggregate fields listed in the `SELECT` statement, removing the need to manually specify each grouping column. This improves query readability and reduces errors in complex queries. ### Syntax ```sql SELECT column1, column2, ..., aggregate_function(...) FROM table GROUP BY ALL ``` ### Behavior - Groups by all visible non-aggregate fields in the `SELECT` list. - Ignores aggregate functions such as `COUNT(*)` and `SUM(...)` when determining grouping columns. - Equivalent to explicitly listing all non-aggregate columns in `GROUP BY`. ### Examples ```sql SELECT x, y, COUNT(*) FROM t1 GROUP BY ALL ``` - Groups by `x` and `y`. ```sql SELECT a, b, c, SUM(d) FROM table GROUP BY ALL ``` - Groups by `a`, `b`, and `c`. ### Limitations - Only works with standard SQL aggregates. - Custom aggregate functions are not automatically skipped. - Requires the hypergraph optimizer to be enabled (default in recent versions). ### Compatibility - Fully backward compatible. - Existing `GROUP BY` queries are unaffected. ### Importance - High-impact feature for SQL usability. - Simplifies query writing for wide tables or dynamic schemas. - Reduces boilerplate and potential mistakes. - Enhances MySQL’s SQL syntax capabilities and alignment with emerging SQL standards. - Suitable for inclusion in “New Features” sections of release notes and documentation.
|
Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment: |
Author
|
"I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it." |
|
Hi, @hlosukwakha Thanks for contributing to MySQL |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR#658:
New Feature:
GROUP BY ALLSupportDescription
MySQL now supports the
GROUP BY ALLclause. It automatically groups by all non-aggregate fields listed in theSELECTstatement, removing the need to manually specify each grouping column. This improves query readability and reduces errors in complex queries.Syntax
Behavior
SELECTlist.COUNT(*)andSUM(...)when determining grouping columns.GROUP BY.Examples
xandy.a,b, andc.Limitations
Compatibility
GROUP BYqueries are unaffected.Importance