[fix](fe) Support dollar sign in mysql pattern#63972
Merged
Merged
Conversation
### What problem does this PR solve? Issue Number: close apache#26112 Related PR: None Problem Summary: Querying information_schema.columns with an external system table name such as table can call FrontendServiceImpl.getTableNames with the table name as a pattern. The MySQL pattern converter rejected '$' as a forbidden regex character, so the FE thrift service threw an internal getTableNames error before metadata resolution. This change treats '$' as a literal character in MySQL patterns by escaping it for the generated Java regex, allowing system table names to be matched safely. ### Release note Fixes metadata lookup for table names containing '$', including external system tables such as table. ### Check List (For Author) - Test: Manual test - Ran git diff --check and git diff --cached --check. - Tried ./run-fe-ut.sh --run org.apache.doris.common.PatternMatcherTest, but the local environment only has JDK 11 and the script requires JDK 17. - Behavior changed: Yes. MySQL pattern matching now allows '$' as a literal table-name character. - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one blocking functional issue in the pattern conversion. The PR goal is valid and focused, but the current escaping path still fails to match table names containing a literal $.
Critical checkpoint conclusions:
- Goal/test: the goal is to allow MySQL-pattern table matching for names containing
$; the added direct unit test covers this, but it would fail with the current conversion. - Scope: the code change is small and focused.
- Concurrency/lifecycle/config/compatibility/persistence/data writes: not applicable; this is FE pattern conversion only.
- Parallel paths:
createMysqlPatternis shared by table-name/list-status matching, so fixing this conversion should cover both downstream paths. - Test coverage: tests were added for
PatternMatcherandFrontendServiceImpl, but the implementation needs adjustment before they can pass. - Observability/performance: not applicable; no new logging or hot-path concern found.
User focus: no additional user-provided review focus was specified.
### What problem does this PR solve? Issue Number: close apache#63972 Related PR: apache#63972 Problem Summary: MySQL pattern conversion escaped '$' in the first pass, but the second pass only preserved escaped '.', so the escaped dollar became a regex that did not match a literal '$'. This change preserves '$' during the second pass so table names containing '$' can be matched literally. ### Release note None ### Check List (For Author) - Test: Manual test - Ran git diff --check and git diff --cached --check. FE unit test org.apache.doris.common.PatternMatcherTest could not run because local JAVA_HOME points to JDK 11 and JDK_17 is not set. - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review result: no additional blocking issues found.
Critical checkpoint conclusions:
- Goal and proof: The PR aims to allow
$in MySQL-style patterns so table names such astest_dropped_partition_field$partitionsdo not fail pattern conversion. The current code removes$from the forbidden set, emits\$, and preserves that escape in the second pass, so the stated goal is met. Tests were added at bothPatternMatcherandFrontendServiceImpl.getTableNameslevels. - Scope and clarity: The functional change is small and focused on
PatternMatcherplus targeted tests. - Concurrency and locks: No new shared mutable state, locking, or concurrent lifecycle behavior is introduced.
- Lifecycle/static initialization: No new static initialization dependencies or lifecycle management risks were introduced.
- Configuration and compatibility: No config, storage format, function symbol, or FE-BE protocol compatibility changes were introduced.
- Parallel paths: The shared
PatternMatcher.createMysqlPatternpath is used by table/db/list/status metadata matching, so the fix applies to the relevant callers rather than only one endpoint. - Conditional checks: The special second-pass preservation for
\$is now aligned with the existing\.handling; no unexplained defensive checks were added. - Test coverage: The added unit coverage validates direct pattern matching and the frontend service call path. I did not run tests in this review environment.
- Test result files: Not applicable.
- Observability: Not applicable for this localized validation fix.
- Transactions/persistence/data writes: Not applicable.
- FE-BE variable passing: Not applicable.
- Performance: No meaningful overhead added.
- Other issues: The previously raised inline issue about
\$being broken in the second pass is already known and appears fixed by the latest commit. I found no distinct additional issue.
User focus: No additional user-provided review focus was specified.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29004 ms |
Contributor
TPC-DS: Total hot run time: 171293 ms |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 29489 ms |
Contributor
TPC-DS: Total hot run time: 170648 ms |
suxiaogang223
approved these changes
Jun 2, 2026
Gabriel39
added a commit
to Gabriel39/incubator-doris
that referenced
this pull request
Jun 2, 2026
### What problem does this PR solve? Issue Number: close apache#63972 Related PR: apache#63972 Problem Summary: Iceberg data location resolution skipped the legacy object-store.path property and fell back directly to write.folder-storage.path or table location when write.data.path was not set. This could choose the wrong data location for tables that still rely on object-store.path. This change keeps write.data.path as the highest priority, then checks object-store.path before write.folder-storage.path and the default table data directory. ### Release note None ### Check List (For Author) - Test: Manual test - Ran git diff --check and git diff --cached --check. FE unit test org.apache.doris.datasource.iceberg.IcebergUtilsTest could not run because local JAVA_HOME points to JDK 11 and JDK_17 is not set. - Behavior changed: No - Does this need documentation: No
16 tasks
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.
What problem does this PR solve?
Issue Number: close #26112
Related PR: None
Problem Summary: Querying information_schema.columns with an external system table name such as table can call FrontendServiceImpl.getTableNames with the table name as a pattern. The MySQL pattern converter rejected '$' as a forbidden regex character, so the FE thrift service threw an internal getTableNames error before metadata resolution. This change treats '$' as a literal character in MySQL patterns by escaping it for the generated Java regex, allowing system table names to be matched safely.
Release note
Fixes metadata lookup for table names containing '$', including external system tables such as table.
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)