You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2026/10/08 23:11:48 UTC
(impala) 01/03: IMPALA-15320: Enforce write capabilities for Iceberg and Kudu DML
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 68ea38cd5f2f2d403351bc5b8a3b5a253dfe17ca
Author: Aleksandr Efimov <dz...@gmail.com>
AuthorDate: Tue Oct 6 21:36:39 2026 +0300
IMPALA-15320: Enforce write capabilities for Iceberg and Kudu DML
Route MERGE, UPDATE, DELETE and OPTIMIZE through the common WRITE
capability check. This enforces the existing bucketed-table and HMS
write restrictions and replaces the REST-only guard.
Keep REST tables limited to INSERT INTO and preserve the Kudu
exception to HMS access-type checks.
Testing:
- Focused Linux Java and fesupport build
- AnalyzerCapabilityTest: 3 tests
- AnalyzeModifyStmtsTest: capabilities, UPDATE and MERGE (4 tests)
- AnalyzeStmtsTest#TestOptimizeNoOp
Change-Id: I7aa7c539a00a0adf8c8c39727e25e274bc98c866
Assisted-by: GPT-6 (Codex)
Reviewed-on: http://gerrit.cloudera.org:8080/25034
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../java/org/apache/impala/analysis/Analyzer.java | 20 ------
.../java/org/apache/impala/analysis/MergeStmt.java | 2 +-
.../org/apache/impala/analysis/ModifyStmt.java | 2 +-
.../org/apache/impala/analysis/OptimizeStmt.java | 2 +-
.../impala/analysis/AnalyzeModifyStmtsTest.java | 75 ++++++++++++++++++++++
.../impala/analysis/AnalyzerCapabilityTest.java | 4 +-
6 files changed, 80 insertions(+), 25 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index affd71d3c..9a784490a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -411,26 +411,6 @@ public class Analyzer {
&& ((FeIcebergTable)table).hasProviderDerivedCapabilities();
}
- /**
- * Refuses every DML other than INSERT INTO on a table whose capabilities come from
- * its provider.
- *
- * <p>Called by hand from MergeStmt, ModifyStmt and OptimizeStmt, because those three
- * do not go through checkTableCapability() at all - only InsertStmt, AlterTableStmt,
- * DropTableOrViewStmt, LoadDataStmt, TruncateStmt and BaseTableRef do. A new DML
- * statement, or a new path into an existing one, needs this call adding. Bringing
- * the three under checkTableCapability() instead is IMPALA-15320: it would start
- * running ensureTableWriteSupported() on statements that have never run it, which
- * is a behaviour change of its own and not one to make here.
- */
- static void ensureNonInsertDmlSupported(FeTable table) throws AnalysisException {
- if (hasProviderDerivedCapabilities(table)) {
- throw new AnalysisException(String.format(
- "Only INSERT INTO is supported for REST-backed Iceberg table: %s",
- table.getFullName()));
- }
- }
-
/** Check whether a table supports this write operation. */
static void ensureTableWriteSupported(FeTable table, boolean isInsertInto)
throws AnalysisException {
diff --git a/fe/src/main/java/org/apache/impala/analysis/MergeStmt.java b/fe/src/main/java/org/apache/impala/analysis/MergeStmt.java
index 725fcd3c3..6fffc3b5b 100644
--- a/fe/src/main/java/org/apache/impala/analysis/MergeStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/MergeStmt.java
@@ -76,7 +76,7 @@ public class MergeStmt extends DmlStatementBase {
targetTableRef_ = analyzer.resolveTableRef(targetTableRef_);
table_ = targetTableRef_.getTable();
- Analyzer.ensureNonInsertDmlSupported(table_);
+ Analyzer.checkTableCapability(table_, Analyzer.OperationType.WRITE);
if (impl_ == null) {
if (table_ instanceof FeIcebergTable) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
index 14daffa62..246ad938f 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
@@ -157,7 +157,7 @@ public abstract class ModifyStmt extends DmlStatementBase {
"but the following table is neither: %s",
dstTbl.getFullName()));
}
- Analyzer.ensureNonInsertDmlSupported(dstTbl);
+ Analyzer.checkTableCapability(dstTbl, Analyzer.OperationType.WRITE);
if (dstTbl instanceof FeIcebergTable) {
setMaxTableSinks(analyzer_.getQueryOptions().getMax_fs_writers());
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java b/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java
index 2d4ec70ab..030f7f9a9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java
@@ -168,7 +168,7 @@ public class OptimizeStmt extends DmlStatementBase {
throw new AnalysisException("OPTIMIZE is only supported for Iceberg tables.");
}
FeIcebergTable iceTable = (FeIcebergTable) table_;
- Analyzer.ensureNonInsertDmlSupported(iceTable);
+ Analyzer.checkTableCapability(iceTable, Analyzer.OperationType.WRITE);
IcebergUtil.validateIcebergTableForInsert(iceTable);
IcebergUtil.warnIfInvalidParquetBloomFilterProperties(iceTable, analyzer);
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
index fff069a32..43905e823 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeModifyStmtsTest.java
@@ -17,6 +17,8 @@
package org.apache.impala.analysis;
+import org.apache.impala.catalog.Table;
+import org.apache.impala.compat.MetastoreShim;
import org.junit.Test;
/**
@@ -27,6 +29,79 @@ import org.junit.Test;
*/
public class AnalyzeModifyStmtsTest extends AnalyzerTest {
+ @Test
+ public void TestIcebergWriteCapabilities() throws Exception {
+ Table table = catalog_.getOrLoadTable("functional_parquet",
+ "iceberg_v2_partitioned_position_deletes");
+ org.apache.hadoop.hive.metastore.api.Table originalMsTable =
+ table.getMetaStoreTable();
+ org.apache.hadoop.hive.metastore.api.Table msTable = originalMsTable.deepCopy();
+ String tableName = table.getFullName();
+ String[] statements = {
+ "update " + tableName + " set id = 1 where id = 2",
+ "delete from " + tableName + " where id = 2",
+ String.format("merge into %s target using (select 1 id) source "
+ + "on target.id = source.id when matched then delete", tableName),
+ "optimize table " + tableName,
+ };
+ try {
+ table.setMetaStoreTable(msTable);
+ msTable.getSd().setNumBuckets(0);
+ if (MetastoreShim.getMajorVersion() > 2) {
+ MetastoreShim.setTableAccessType(msTable, Analyzer.ACCESSTYPE_READWRITE);
+ }
+ for (String statement : statements) AnalyzesOk(statement);
+
+ if (MetastoreShim.getMajorVersion() > 2) {
+ MetastoreShim.setTableAccessType(msTable, Analyzer.ACCESSTYPE_READ);
+ for (String statement : statements) {
+ AnalysisError(statement, "Write not supported. Table " + tableName);
+ }
+ MetastoreShim.setTableAccessType(msTable, Analyzer.ACCESSTYPE_READWRITE);
+ }
+
+ msTable.getSd().setNumBuckets(1);
+ for (String statement : statements) {
+ AnalysisError(statement,
+ tableName + " is a bucketed table. Only read operations are supported "
+ + "on such tables.");
+ }
+ } finally {
+ table.setMetaStoreTable(originalMsTable);
+ }
+ }
+
+ @Test
+ public void TestKuduWriteCapabilities() throws Exception {
+ Table table = catalog_.getOrLoadTable("functional_kudu", "dimtbl");
+ org.apache.hadoop.hive.metastore.api.Table originalMsTable =
+ table.getMetaStoreTable();
+ org.apache.hadoop.hive.metastore.api.Table msTable = originalMsTable.deepCopy();
+ String tableName = table.getFullName();
+ String[] statements = {
+ "update " + tableName + " set name = 'test' where id = 1",
+ "delete from " + tableName + " where id = 1",
+ };
+ try {
+ table.setMetaStoreTable(msTable);
+ msTable.getSd().setNumBuckets(0);
+ // Kudu writes do not require HMS write capabilities.
+ if (MetastoreShim.getMajorVersion() > 2) {
+ MetastoreShim.setTableAccessType(msTable, Analyzer.ACCESSTYPE_READ);
+ }
+ for (String statement : statements) AnalyzesOk(statement);
+
+ msTable.getSd().setNumBuckets(1);
+ for (String statement : statements) {
+ AnalysisError(statement,
+ tableName + " is a bucketed table. Only read operations are supported "
+ + "on such tables.");
+ }
+ } finally {
+ table.setMetaStoreTable(originalMsTable);
+ }
+ }
+
@Test
public void TestFromListAliases() {
AnalysisError("update a.name set a.name = 'Oskar' from functional_kudu.testtbl a",
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzerCapabilityTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzerCapabilityTest.java
index e6ba5721a..145119d32 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzerCapabilityTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzerCapabilityTest.java
@@ -53,7 +53,7 @@ public class AnalyzerCapabilityTest {
Analyzer.ensureTableWriteSupported(table, true);
try {
- Analyzer.ensureNonInsertDmlSupported(table);
+ Analyzer.checkTableCapability(table, Analyzer.OperationType.WRITE);
fail("Expected non-INSERT DML to be rejected");
} catch (AnalysisException e) {
assertTrue(e.getMessage(),
@@ -92,7 +92,7 @@ public class AnalyzerCapabilityTest {
private static void assertRejectedNonInsertDml(
FeTable table, String expectedMessage) throws Exception {
try {
- Analyzer.ensureNonInsertDmlSupported(table);
+ Analyzer.checkTableCapability(table, Analyzer.OperationType.WRITE);
fail("Expected non-INSERT DML to be rejected");
} catch (AnalysisException e) {
assertTrue(e.getMessage(), e.getMessage().contains(expectedMessage));