Skip to content

[fix](auth) delete command improperly detects the LOAD permission of non target tables #60837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
liutang123 wants to merge 7 commits into
base: master
Choose a base branch
from
+108 −26
Open

[fix](auth) delete command improperly detects the LOAD permission of non target tables #60837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2de893c
[fix](auth) Check target table's load privilage
Jan 28, 2026
fe3663b
optmize
Feb 25, 2026
0265991
ptimize error msg
Feb 25, 2026
1a4ea63
ptimize error msg 2
Feb 25, 2026
bf83ff5
ptimize error msg 3
Feb 25, 2026
70fad14
add rt
Feb 25, 2026
e8d35bf
fix rt
Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 29 additions & 24 deletions ...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DeleteFromCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.Config;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.util.Util;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.CascadesContext;
Expand Down Expand Up @@ -149,36 +150,27 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
.getDeleteHandler().processEmptyRelation(ctx.getState());
return;
}
OlapTable olapTable = getTargetTable(ctx);

// check auth
if (!Env.getCurrentEnv().getAccessManager()
.checkTblPriv(ConnectContext.get(), olapTable.getDatabase().getCatalog().getName(),
olapTable.getDatabase().getFullName(), olapTable.getName(), PrivPredicate.LOAD)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR, "LOAD",
ConnectContext.get().getQualifiedUser(), ConnectContext.get().getRemoteIP(),
olapTable.getDatabase().getFullName() + "." + Util.getTempTableDisplayName(olapTable.getName()));
}

Optional<PhysicalFilter<?>> optFilter = (planner.getPhysicalPlan()
.<PhysicalFilter<?>>collect(PhysicalFilter.class::isInstance)).stream()
.findAny();
Optional<PhysicalOlapScan> optScan = (planner.getPhysicalPlan()
.<PhysicalOlapScan>collect(PhysicalOlapScan.class::isInstance)).stream()
.findAny();
Optional<UnboundRelation> optRelation = (logicalQuery
.<UnboundRelation>collect(UnboundRelation.class::isInstance)).stream()
.findAny();
Preconditions.checkArgument(optFilter.isPresent(), "delete command must contain filter");
Preconditions.checkArgument(optScan.isPresent(), "delete command could be only used on olap table");
Preconditions.checkArgument(optRelation.isPresent(), "delete command could be only used on olap table");
PhysicalOlapScan scan = optScan.get();
UnboundRelation relation = optRelation.get();
PhysicalFilter<?> filter = optFilter.get();

if (!Env.getCurrentEnv().getAccessManager()
.checkTblPriv(ConnectContext.get(), scan.getDatabase().getCatalog().getName(),
scan.getDatabase().getFullName(),
scan.getTable().getName(), PrivPredicate.LOAD)) {
String message = ErrorCode.ERR_TABLEACCESS_DENIED_ERROR.formatErrorMsg("LOAD",
ConnectContext.get().getQualifiedUser(), ConnectContext.get().getRemoteIP(),
scan.getDatabase().getFullName() + ": " + Util.getTempTableDisplayName(scan.getTable().getName()));
throw new AnalysisException(message);
}

// predicate check
OlapTable olapTable = scan.getTable();
Set<String> columns = olapTable.getFullSchema().stream().map(Column::getName).collect(Collectors.toSet());
try {
// treat sql as simple `delete from t where keyC = ...`
Plan plan = planner.getPhysicalPlan();
checkSubQuery(plan);
for (Expression conjunct : filter.getConjuncts()) {
Expand All @@ -192,14 +184,16 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
logicalQuery, Optional.empty()).run(ctx, executor);
return;
} catch (Exception e2) {
LOG.warn("delete from command failed", e2);
throw e;
}
}

// if table's enable_mow_light_delete is false, use `DeleteFromUsingCommand`
if (olapTable.getKeysType() == KeysType.UNIQUE_KEYS && olapTable.getEnableUniqueKeyMergeOnWrite()
&& !olapTable.getEnableMowLightDelete()) {
new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions,
logicalQuery, Optional.empty()).run(ctx, executor);
new DeleteFromUsingCommand(nameParts, tableAlias, isTempPart, partitions, logicalQuery,
Optional.empty()).run(ctx, executor);
return;
}

Expand All @@ -225,6 +219,17 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
throw new AnalysisException("delete all rows is forbidden temporary.");
}

Optional<UnboundRelation> optRelation = (logicalQuery
.<UnboundRelation>collect(UnboundRelation.class::isInstance)).stream()
.findAny();
Optional<PhysicalOlapScan> optScan = (planner.getPhysicalPlan()
.<PhysicalOlapScan>collect(PhysicalOlapScan.class::isInstance)).stream()
.findAny();
Preconditions.checkArgument(optRelation.isPresent(), "delete command must apply to one table");
Preconditions.checkArgument(optScan.isPresent(), "delete command could be only used on olap table");
// prune partitions
PhysicalOlapScan scan = optScan.get();
UnboundRelation relation = optRelation.get();
ArrayList<String> partitionNames = Lists.newArrayList(relation.getPartNames());
List<Partition> selectedPartitions = getSelectedPartitions(olapTable, filter, scan, partitionNames);

Expand Down Expand Up @@ -459,7 +464,7 @@ private OlapTable getTargetTable(ConnectContext ctx) {
List<String> qualifiedTableName = RelationUtil.getQualifierName(ctx, nameParts);
TableIf table = RelationUtil.getTable(qualifiedTableName, ctx.getEnv(), Optional.empty());
if (!(table instanceof OlapTable)) {
throw new AnalysisException("table must be olapTable in delete command");
throw new AnalysisException("delete command could be only used on olap table");
}
return ((OlapTable) table);
}
Expand Down
81 changes: 79 additions & 2 deletions regression-test/suites/auth_call/test_dml_delete_table_auth.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ suite("test_dml_delete_table_auth","p0,auth_call") {
String pwd = 'C123_567p'
String dbName = 'test_dml_delete_table_auth_db'
String tableName = 'test_dml_delete_table_auth_tb'
String fromTableName = 'test_dml_delete_table_auth_from_tb'

try_sql("DROP USER ${user}")
try_sql """drop database if exists ${dbName}"""
Expand All @@ -42,9 +43,11 @@ suite("test_dml_delete_table_auth","p0,auth_call") {
id BIGINT,
username VARCHAR(20)
)
UNIQUE KEY(`id`)
DISTRIBUTED BY HASH(id) BUCKETS 2
PROPERTIES (
"replication_num" = "1"
"replication_num" = "1",
"enable_mow_light_delete" = "true"
);"""
sql """
insert into ${dbName}.`${tableName}` values
Expand All @@ -53,6 +56,46 @@ suite("test_dml_delete_table_auth","p0,auth_call") {
(3, "333");
"""

sql """create table ${dbName}.${fromTableName} (
id BIGINT,
username VARCHAR(20)
)
DISTRIBUTED BY HASH(id) BUCKETS 2
PROPERTIES (
"replication_num" = "1"
);"""
sql """
insert into ${dbName}.`${fromTableName}` values
(1, "111"),
(2, "222"),
(3, "333");
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
insert into ${dbName}.`${fromTableName}` select * from ${dbName}.${fromTableName};
"""
sql """
analyze table ${dbName}.`${fromTableName}` WITH SYNC;
"""

// 1. delete when no privilege /////////////////////////////////////////////////
connect(user, "${pwd}", context.config.jdbcUrl) {
test {
sql """DELETE FROM ${dbName}.${tableName} WHERE id = 3;"""
Expand All @@ -62,16 +105,49 @@ suite("test_dml_delete_table_auth","p0,auth_call") {
def del_res = sql """show DELETE from ${dbName}"""
assertTrue(del_res.size() == 0)
}
Thread.sleep(70000) // wait for row count report of the tables just loaded, optimizer will choose right anti join
connect(user, "${pwd}", context.config.jdbcUrl) {
test {
sql """DELETE FROM ${dbName}.${tableName}
WHERE NOT EXISTS
(
SELECT 1 FROM ${dbName}.${fromTableName}
WHERE ${dbName}.${fromTableName}.id = ${dbName}.${tableName}.id
AND ${dbName}.${fromTableName}.username = '333'
);"""
// LOAD command denied to user xx for table '$dbName.$tableName'
exception tableName
}
checkNereidsExecute("show DELETE from ${dbName}")
def del_res = sql """show DELETE from ${dbName}"""
assertTrue(del_res.size() == 0)
}

// 2. delete when has load privilege /////////////////////////////////////////////////
sql """grant load_priv on ${dbName}.${tableName} to ${user}"""
connect(user, "${pwd}", context.config.jdbcUrl) {
sql """DELETE FROM ${dbName}.${tableName} WHERE id = 3;"""
def del_res = sql """show DELETE from ${dbName}"""
logger.info("del_res: " + del_res)
assertTrue(del_res.size() == 1)
}

def res = sql """select count(*) from ${dbName}.${tableName};"""
assertTrue(res[0][0] == 2)

connect(user, "${pwd}", context.config.jdbcUrl) {
sql """DELETE FROM ${dbName}.${tableName}
WHERE NOT EXISTS
(
SELECT 1 FROM ${dbName}.${fromTableName}
WHERE ${dbName}.${fromTableName}.id = ${dbName}.${tableName}.id
AND ${dbName}.${fromTableName}.username = '111'
);"""
def del_res = sql """show DELETE from ${dbName}"""
assertTrue(del_res.size() == 1) // use delete from using
}

def res2 = sql """select count(*) from ${dbName}.${tableName};"""
assertTrue(res2[0][0] == 1)

String tableName1 = 'test_dml_delete_table_auth_tb1'
String tableName2 = 'test_dml_delete_table_auth_tb2'
Expand Down Expand Up @@ -139,3 +215,4 @@ suite("test_dml_delete_table_auth","p0,auth_call") {
sql """drop database if exists ${dbName}"""
try_sql("DROP USER ${user}")
}