[CALCITE-5681] Support authorization via GRANT and REVOKE DDL commands #3726
Conversation
|
| "org.apache.calcite.sql.SqlCreate" | ||
| "org.apache.calcite.sql.SqlDrop" | ||
| "org.apache.calcite.sql.SqlTruncate" | ||
| "org.apache.calcite.sql.ddl.SqlCreateTableLike" | ||
| "org.apache.calcite.sql.ddl.SqlDdlNodes" | ||
| "org.apache.calcite.sql.ddl.SqlAuthCommand" |
There was a problem hiding this comment.
maintain alphabetical order?
| truncateStatementParserMethods: [ | ||
| "SqlTruncateTable" | ||
| ] | ||
|
|
There was a problem hiding this comment.
If it is similar to others, also add corresponding remarks.
|
|
||
| @Test void testGrantForSchema() { | ||
| final String sql = "grant update, delete on all tables in schema s1 to u1, u2"; | ||
| final String expected = "GRANT UPDATE, DELETE ON ALL TABLES IN SCHEMA `S1` TO `U1`, `U2`"; |
There was a problem hiding this comment.
Add more unit tests for different scenarios, like 'ALL TABLES IN ROOT SCHEMA'
| grantStatement: | ||
| GRANT | ||
| { | ||
| privilege [, privilege ]* |
There was a problem hiding this comment.
Can describe the types of privilege
| REVOKE | ||
| { | ||
| privilege [, privilege ]* | ||
| | ALL [ PRIVILEGES ] |
There was a problem hiding this comment.
Small question: although Postgres supports this syntax, does calcite require this flexible syntax, as most databases handle it strictly?
Using a stricter syntax makes it less selective for the user and can be more explicit for the user.
| } | ||
| ON | ||
| { | ||
| [ TABLE ] table [, table ]* |
There was a problem hiding this comment.
- Are only the
TABLEtype currently considered for resource types? Is this syntax more generic, such as usingresource_type, which lists supported types:TABLE - Can
ALL TABLES IN SCHEMAbe equivalently represented as theSCHEMAresource type?
|
|
||
| #### Granting and revoking privileges | ||
|
|
||
| You can grant and revoke privileges on tables, views and materialized views. |
There was a problem hiding this comment.
'tables, views and materialized views', do all their types use TABLE?
| * that represents a user. CalcitePrincipal ignore the case of the | ||
| * name when comparing two principals. | ||
| */ | ||
| public class CalcitePrincipal implements Principal, Comparable<CalcitePrincipal> { |
There was a problem hiding this comment.
To refer to a user can use UserPrincipal
| @@ -71,6 +74,9 @@ public abstract class CalciteSchema { | |||
| protected final NameSet functionNames; | |||
| protected final NameMap<FunctionEntry> nullaryFunctionMap; | |||
| protected final NameMap<CalciteSchema> subSchemaMap; | |||
|
|
|||
| // tableName -> (user, access) | |||
| protected final NameMap<Map<Principal, SqlAccessType>> privilegeMap; | |||
There was a problem hiding this comment.
Whether to use a separate class to maintain the relationship between Principal, SqlAccessType, and resource_type(TABLE/SCHEMA/FUNCTION...).
Using this way resource types can be more general and facilitate subsequent expansion.
CALCITE-5681