YiwenWu commented on code in PR #3726:
URL: https://github.com/apache/calcite/pull/3726#discussion_r1531402290
##########
site/_docs/reference.md:
##########
@@ -3542,6 +3544,34 @@ dropFunctionStatement:
truncateTableStatement:
TRUNCATE TABLE name
[ CONTINUE IDENTITY | RESTART IDENTITY ]
+
+grantStatement:
+ GRANT
+ {
+ privilege [, privilege ]*
+ | ALL [ PRIVILEGES ]
+ }
+ ON
+ {
+ [ TABLE ] table [, table ]*
+ | ALL TABLES IN SCHEMA schema [, schema ]*
+ | ALL TABLES IN ROOT SCHEMA
+ }
+ TO user [, user ]*
+
+revokeStatement:
+ REVOKE
+ {
+ privilege [, privilege ]*
+ | ALL [ PRIVILEGES ]
+ }
+ ON
+ {
+ [ TABLE ] table [, table ]*
Review Comment:
1. Are only the `TABLE` type currently considered for resource types? Is
this syntax more generic, such as using `resource_type`, which lists supported
types: `TABLE`
2. Can `ALL TABLES IN SCHEMA` be equivalently represented as the `SCHEMA`
resource type?
##########
core/src/main/java/org/apache/calcite/access/CalcitePrincipal.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.access;
+
+import org.apache.commons.lang.StringUtils;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.security.Principal;
+import java.util.Locale;
+
+import static org.apache.calcite.runtime.Utilities.hash;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * CalcitePrincipal is a simple implementation of {@link Principal}
+ * that represents a user. CalcitePrincipal ignore the case of the
+ * name when comparing two principals.
+ */
+public class CalcitePrincipal implements Principal,
Comparable<CalcitePrincipal> {
Review Comment:
To refer to a user can use `UserPrincipal`
##########
core/src/main/java/org/apache/calcite/jdbc/CalciteSchema.java:
##########
@@ -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;
Review 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.
##########
site/_docs/reference.md:
##########
@@ -3542,6 +3544,34 @@ dropFunctionStatement:
truncateTableStatement:
TRUNCATE TABLE name
[ CONTINUE IDENTITY | RESTART IDENTITY ]
+
+grantStatement:
+ GRANT
+ {
+ privilege [, privilege ]*
Review Comment:
Can describe the types of `privilege`
##########
site/_docs/reference.md:
##########
@@ -3542,6 +3544,34 @@ dropFunctionStatement:
truncateTableStatement:
TRUNCATE TABLE name
[ CONTINUE IDENTITY | RESTART IDENTITY ]
+
+grantStatement:
+ GRANT
+ {
+ privilege [, privilege ]*
+ | ALL [ PRIVILEGES ]
+ }
+ ON
+ {
+ [ TABLE ] table [, table ]*
+ | ALL TABLES IN SCHEMA schema [, schema ]*
+ | ALL TABLES IN ROOT SCHEMA
+ }
+ TO user [, user ]*
+
+revokeStatement:
+ REVOKE
+ {
+ privilege [, privilege ]*
+ | ALL [ PRIVILEGES ]
Review 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.
##########
server/src/main/codegen/config.fmpp:
##########
@@ -84,12 +90,17 @@ data: {
"SqlDropFunction"
]
- # List of methods for parsing extensions to "TRUNCATE" calls.
- # Each must accept arguments "(SqlParserPos pos)".
- # Example: "SqlTruncateTable".
- truncateStatementParserMethods: [
- "SqlTruncateTable"
- ]
+ # List of methods for parsing extensions to "TRUNCATE" calls.
+ # Each must accept arguments "(SqlParserPos pos)".
+ # Example: "SqlTruncateTable".
+ truncateStatementParserMethods: [
+ "SqlTruncateTable"
+ ]
+
Review Comment:
If it is similar to others, also add corresponding remarks.
##########
server/src/main/codegen/config.fmpp:
##########
@@ -27,11 +27,13 @@ data: {
# List of import statements.
imports: [
"org.apache.calcite.schema.ColumnStrategy"
+ "org.apache.calcite.sql.SqlAccessEnum"
"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"
Review Comment:
maintain alphabetical order?
##########
server/src/test/java/org/apache/calcite/test/ServerParserTest.java:
##########
@@ -362,4 +362,28 @@ class ServerParserTest extends SqlParserTest {
sql(sql).ok(expected);
}
+ @Test void testGrant() {
+ final String sql = "grant select, insert on x1.y1, y2 to u1, u2";
+ final String expected = "GRANT SELECT, INSERT ON `X1`.`Y1`, `Y2` TO `U1`,
`U2`";
+ sql(sql).ok(expected);
+ }
+
+ @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`";
Review Comment:
Add more unit tests for different scenarios, like 'ALL TABLES IN ROOT SCHEMA'
##########
site/_docs/reference.md:
##########
@@ -3595,3 +3625,31 @@ employee_typ(315, 'Francis', 'Logan', 'FLOGAN',
'555.777.2222', DATE '2004-05-01', 'SA_MAN', 11000, .15, 101, 110,
address_typ('376 Mission', 'San Francisco', 'CA', '94222'))
{% endhighlight %}
+
+#### Granting and revoking privileges
+
+You can grant and revoke privileges on tables, views and materialized views.
Review Comment:
'tables, views and materialized views', do all their types use `TABLE`?
--
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]
