924060929 commented on code in PR #13879: URL: https://github.com/apache/doris/pull/13879#discussion_r1030498380
########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java: ########## @@ -0,0 +1,177 @@ +// 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.doris.nereids.trees.plans.logical; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.Database; +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.memo.GroupExpression; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.trees.expressions.And; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.PlanType; +import org.apache.doris.nereids.trees.plans.commands.CreatePolicyCommand; +import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; +import org.apache.doris.nereids.util.ExpressionUtils; +import org.apache.doris.nereids.util.Utils; +import org.apache.doris.policy.PolicyMgr; +import org.apache.doris.policy.RowPolicy; +import org.apache.doris.qe.ConnectContext; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +/** + * Logical Check Policy + */ +public class LogicalCheckPolicy<CHILD_TYPE extends Plan> extends LogicalUnary<CHILD_TYPE> { + + public LogicalCheckPolicy(CHILD_TYPE child) { + super(PlanType.LOGICAL_CHECK_POLICY, child); + } + + public LogicalCheckPolicy(Optional<GroupExpression> groupExpression, + Optional<LogicalProperties> logicalProperties, CHILD_TYPE child) { + super(PlanType.LOGICAL_CHECK_POLICY, groupExpression, logicalProperties, child); + } + + @Override + public <R, C> R accept(PlanVisitor<R, C> visitor, C context) { + return visitor.visitLogicalCheckPolicy(this, context); + } + + @Override + public List<? extends Expression> getExpressions() { + return ImmutableList.of(); + } + + @Override + public List<Slot> computeOutput() { + return child().getOutput(); + } + + @Override + public String toString() { + return Utils.toSqlString("LogicalCheckPolicy", + "child", child() + ); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LogicalCheckPolicy that = (LogicalCheckPolicy) o; + return child().equals(that.child()); + } + + @Override + public int hashCode() { + return child().hashCode(); + } + + @Override + public Plan withGroupExpression(Optional<GroupExpression> groupExpression) { + return new LogicalCheckPolicy<>(groupExpression, Optional.of(getLogicalProperties()), child()); + } + + @Override + public Plan withLogicalProperties(Optional<LogicalProperties> logicalProperties) { + return new LogicalCheckPolicy<>(Optional.empty(), logicalProperties, child()); + } + + @Override + public Plan withChildren(List<Plan> children) { + Preconditions.checkArgument(children.size() == 1); + return new LogicalCheckPolicy<>(children.get(0)); + } + + /** + * get wherePredicate of policy for logicalRelation. + * + * @param logicalRelation include tableName and dbName + * @param connectContext include information about user and policy + */ + public Optional<Expression> getFilter(LogicalRelation logicalRelation, ConnectContext connectContext) { + String dbName = !logicalRelation.getQualifier().isEmpty() ? logicalRelation.getQualifier().get(0) : null; + Database db = connectContext.getEnv().getInternalCatalog().getDb(dbName) + .orElseThrow(() -> new RuntimeException("Database [" + dbName + "] does not exist.")); + long dbId = db.getId(); + long tableId = logicalRelation.getTable().getId(); + + PolicyMgr policyMgr = connectContext.getEnv().getPolicyMgr(); + UserIdentity currentUserIdentity = connectContext.getCurrentUserIdentity(); + String user = connectContext.getQualifiedUser(); + if (currentUserIdentity.isRootUser() || currentUserIdentity.isAdminUser()) { + return Optional.empty(); + } + if (!policyMgr.existPolicy(user)) { + return Optional.empty(); + } + + List<RowPolicy> policies = policyMgr.getMatchRowPolicy(dbId, tableId, currentUserIdentity); + if (policies.isEmpty()) { + return Optional.empty(); + } + return Optional.of(mergeRowPolicy(policies)); + } + + private Expression mergeRowPolicy(List<RowPolicy> policies) { + List<Expression> orList = new ArrayList<>(); + List<Expression> andList = new ArrayList<>(); + for (RowPolicy policy : policies) { + String sql = policy.getOriginStmt(); + NereidsParser nereidsParser = new NereidsParser(); + CreatePolicyCommand command = (CreatePolicyCommand) nereidsParser.parseSingle(sql); + Optional<Expression> wherePredicate = command.getWherePredicate(); + if (!wherePredicate.isPresent()) { + throw new AnalysisException("Invaild row policy [" + policy.getPolicyName() + "], " + sql); + } + switch (policy.getFilterType()) { + case PERMISSIVE: + orList.add(wherePredicate.get()); + break; + case RESTRICTIVE: + andList.add(wherePredicate.get()); + break; + default: + throw new IllegalStateException("Invalid operator"); + } + } + if (!andList.isEmpty() && !orList.isEmpty()) { + return new And(ExpressionUtils.and(andList), ExpressionUtils.or(orList)); + } else if (andList.isEmpty()) { + return ExpressionUtils.or(orList); + } else if (orList.isEmpty()) { + return ExpressionUtils.and(andList); + } else { + return null; Review Comment: `Optional.of(null)`will throw exception ########## regression-test/suites/account_p0/test_nereids_row_policy.groovy: ########## @@ -0,0 +1,91 @@ +// 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. +suite("test_nereids_row_policy") { + def dbName = context.config.getDbNameByFile(context.file) + def tableName = "nereids_row_policy" + def user='row_policy_user' + def tokens = context.config.jdbcUrl.split('/') + def url=tokens[0] + "//" + tokens[2] + "/" + dbName + "?" + + def assertQueryResult = { size -> + def result1 = connect(user=user, password='123456', url=url) { + sql "set enable_nereids_planner = false" + sql "SELECT * FROM ${tableName}" + } + def result2 = connect(user=user, password='123456', url=url) { + sql "set enable_nereids_planner = true" + sql "SELECT * FROM ${tableName}" + } + assertEquals(size, result1.size()) + assertEquals(size, result2.size()) + } + + def createPolicy = { name, predicate, type -> + sql """ + CREATE ROW POLICY ${name} ON ${dbName}.${tableName} + AS ${type} TO ${user} USING (${predicate}) + """ + } + + def dropPolciy = { name -> + sql """ + DROP ROW POLICY ${name} ON ${dbName}.${tableName} FOR ${user} + """ + } + + // create table + sql "DROP TABLE IF EXISTS ${tableName}" + sql """ + CREATE TABLE ${tableName} ( + `k` INT, + `v` INT + ) DUPLICATE KEY (`k`) DISTRIBUTED BY HASH (`k`) BUCKETS 1 + PROPERTIES ('replication_num' = '1') + """ + sql """ + insert into ${tableName} values (1,1), (2,1), (1,3); + """ + // create user + sql "DROP USER IF EXISTS ${user}" + sql "CREATE USER ${user} IDENTIFIED BY '123456'" + sql "GRANT SELECT_PRIV ON internal.${dbName}.${tableName} TO ${user}" + + + // no policy + assertQueryResult 3 + + // (k = 1) + createPolicy"policy0", "k = 1", "RESTRICTIVE" + assertQueryResult 2 + + // (k = 1 and v = 1) + createPolicy"policy1", "v = 1", "RESTRICTIVE" + assertQueryResult 1 + + // (v = 1) + dropPolciy "policy0" + assertQueryResult 2 + + // (v = 1) and (k = 1) + createPolicy"policy2", "k = 1", "PERMISSIVE" + assertQueryResult 1 + + // (v = 1) and (k = 1 or k = 2) + createPolicy"policy3", "k = 2", "PERMISSIVE" + assertQueryResult 2 + Review Comment: we should check row policy whether is valid in a view ########## fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java: ########## @@ -510,6 +510,8 @@ private void eliminateFromGroupAndMoveToTargetGroup(Group fromGroup, Group targe "TargetGroup should be ancestors of fromGroup, but fromGroup is root. Maybe a bug"); } + moveParentExpressionsReference(fromGroup, targetGroup); Review Comment: In the context, `fromGroup` is the group of `LogicalRelation`, and the `targetGroup` is the group of `LogicalRowPolicy`, `eliminateFromGroupAndMoveToTargetGroup` will move the logicalExpression(LogicalRowPolicy) to the targetGroup. And the origin logicalExpression in the targetGroup will be recycle until the `LogicalRelation`. So I don't think we should `moveParentExpressionsReference(fromGroup, targetGroup)` ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalCheckPolicy.java: ########## @@ -0,0 +1,177 @@ +// 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.doris.nereids.trees.plans.logical; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.Database; +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.memo.GroupExpression; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.trees.expressions.And; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.PlanType; +import org.apache.doris.nereids.trees.plans.commands.CreatePolicyCommand; +import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; +import org.apache.doris.nereids.util.ExpressionUtils; +import org.apache.doris.nereids.util.Utils; +import org.apache.doris.policy.PolicyMgr; +import org.apache.doris.policy.RowPolicy; +import org.apache.doris.qe.ConnectContext; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +/** + * Logical Check Policy + */ +public class LogicalCheckPolicy<CHILD_TYPE extends Plan> extends LogicalUnary<CHILD_TYPE> { + + public LogicalCheckPolicy(CHILD_TYPE child) { + super(PlanType.LOGICAL_CHECK_POLICY, child); + } + + public LogicalCheckPolicy(Optional<GroupExpression> groupExpression, + Optional<LogicalProperties> logicalProperties, CHILD_TYPE child) { + super(PlanType.LOGICAL_CHECK_POLICY, groupExpression, logicalProperties, child); + } + + @Override + public <R, C> R accept(PlanVisitor<R, C> visitor, C context) { + return visitor.visitLogicalCheckPolicy(this, context); + } + + @Override + public List<? extends Expression> getExpressions() { + return ImmutableList.of(); + } + + @Override + public List<Slot> computeOutput() { + return child().getOutput(); + } + + @Override + public String toString() { + return Utils.toSqlString("LogicalCheckPolicy", + "child", child() + ); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + LogicalCheckPolicy that = (LogicalCheckPolicy) o; + return child().equals(that.child()); + } + + @Override + public int hashCode() { + return child().hashCode(); + } + + @Override + public Plan withGroupExpression(Optional<GroupExpression> groupExpression) { + return new LogicalCheckPolicy<>(groupExpression, Optional.of(getLogicalProperties()), child()); + } + + @Override + public Plan withLogicalProperties(Optional<LogicalProperties> logicalProperties) { + return new LogicalCheckPolicy<>(Optional.empty(), logicalProperties, child()); + } + + @Override + public Plan withChildren(List<Plan> children) { + Preconditions.checkArgument(children.size() == 1); + return new LogicalCheckPolicy<>(children.get(0)); + } + + /** + * get wherePredicate of policy for logicalRelation. + * + * @param logicalRelation include tableName and dbName + * @param connectContext include information about user and policy + */ + public Optional<Expression> getFilter(LogicalRelation logicalRelation, ConnectContext connectContext) { + String dbName = !logicalRelation.getQualifier().isEmpty() ? logicalRelation.getQualifier().get(0) : null; + Database db = connectContext.getEnv().getInternalCatalog().getDb(dbName) + .orElseThrow(() -> new RuntimeException("Database [" + dbName + "] does not exist.")); Review Comment: The `LogicalTVFRelation` is `LogicalRelation` but should not throw exception. I think we should provide some interfaces, like `CatalogRelation`, `InternalCatalogRelation`, `ExternalCatalogRelation`, then we check for `CatalogRelation` -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org
