CalvinKirs opened a new pull request, #68660:
URL: https://github.com/apache/doris/pull/68660
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
The grammar of table privilege statements accepts an object name with any
number of dot-separated parts (`multipartIdentifierOrAsterisk`), but
`LogicalPlanBuilder` only turned 1, 2 or 3 parts into a `TablePattern` (db,
db.tbl, ctl.db.tbl) and left it null for anything else. The null pattern then
reached the commands and surfaced as an internal error:
```sql
GRANT SELECT_PRIV ON a.b.c.d TO 'u'@'%';
-- errCode = 2, detailMessage = tablePattern is null
REVOKE SELECT_PRIV ON a.b.c.d FROM 'u'@'%';
-- errCode = 2, detailMessage = Cannot invoke
"org.apache.doris.analysis.TablePattern.analyze()" because "this.tablePattern"
is null
```
GRANT failed on the `Objects.requireNonNull` in the
`GrantTablePrivilegeCommand` constructor; REVOKE had no such check and hit a
NullPointerException in `validate()`. For comparison, MySQL rejects the same
statements as a syntax error (`ERROR 1064 ... near '.c.d TO ...'`).
Fix:
- Build the `TablePattern` of both statements in one helper,
`parsePrivilegeTablePattern`, which throws a `ParseException` naming the
accepted shapes for any other part count:
```
Privilege object name should be db, db.tbl or ctl.db.tbl, but got:
a.b.c.d(line 1, pos 21)
== SQL ==
GRANT SELECT_PRIV ON a.b.c.d TO 'u'@'%'
---------------------^^^
```
- `RevokeTablePrivilegeCommand` now rejects null arguments in its
constructor, the same as `GrantTablePrivilegeCommand`. The `tablePattern !=
null` checks in `RevokeTablePrivilegeCommand.validate()` and
`Auth.revokeTablePrivilegeCommand()` are removed since the pattern can no
longer be null; the one in `Auth` would have skipped the revoke silently
instead of failing.
### Release note
GRANT/REVOKE with a table privilege object name of more than three parts
(for example `a.b.c.d`) now fails with the parse error "Privilege object name
should be db, db.tbl or ctl.db.tbl" instead of an internal null pointer error.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- FE UT: `GrantTablePrivilegeCommandTest` and
`RevokeTablePrivilegeCommandTest` gain `testObjectName` (1/2/3-part mapping)
and `testObjectNameWithTooManyParts` (4/5 parts and `*.*.*.*` rejected); the
rejection cases fail without the fix. 14 privilege and parser test classes (63
tests) pass.
- Regression: new `account_p0/test_grant_revoke_object_name`, run on a
local cluster with the FE built from this branch.
- Manual: the statements above, plus `a.b.c.d.e`, `` `a`.`b`.`c`.`d` ``
and `GRANT ... TO ROLE`, all return the parse error; `a.b.c` still goes through
the normal path.
- Behavior changed:
- [ ] No.
- [x] Yes. A 4+ part object name in GRANT/REVOKE now fails at parse time
with the error above instead of a null pointer message. Valid statements are
unchanged.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
