[
https://issues.apache.org/jira/browse/IMPALA-15320?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109936#comment-18109936
]
Aleksandr Efimov commented on IMPALA-15320:
-------------------------------------------
Measured what this would break before writing it.
Added analyzer.checkTableCapability(table_, OperationType.WRITE) to MergeStmt,
ModifyStmt and OptimizeStmt, right after each sets its target table, and ran
six analyzer suites before and after: AnalyzeModifyStmtsTest, MergeInsertTest,
AnalyzeStmtsTest, AnalyzeDDLTest, AnalyzeKuduDDLTest, AuditingKuduTest.
The two runs come out identical: 191 tests, 81 failures both times, the same 59
test names. Nothing new fails and nothing starts passing. MergeInsertTest is
green end to end in both, so the MERGE path is genuinely exercised.
Those 81 failures are the tree rather than the check - they are "Could not
resolve table reference" on functional_kudu.alltypes, functional_kudu.testtbl,
functional_hbase.alltypes and functional.alltypes_datasource, none of which
were loaded there. So the Kudu paths went untested. That is the least worrying
part: ensureTableWriteSupported() returns early for Kudu right after
ensureTableNotBucketed(), so a Kudu table only ever meets the bucketed check.
What stays a real behaviour change is a table whose HMS access type is READ
that accepts UPDATE, DELETE, MERGE or OPTIMIZE today - which is what this issue
wants to stop. So the scope looks like the three calls, dropping the
hand-written ensureNonInsertDmlSupported() that IMPALA-15310 adds, and tests
for the new refusal.
> Bring MERGE, UPDATE, DELETE and OPTIMIZE under checkTableCapability()
> ---------------------------------------------------------------------
>
> Key: IMPALA-15320
> URL: https://issues.apache.org/jira/browse/IMPALA-15320
> Project: IMPALA
> Issue Type: Improvement
> Components: Frontend
> Reporter: Aleksandr Efimov
> Assignee: Aleksandr Efimov
> Priority: Major
>
> Analyzer.checkTableCapability() is where a statement asks whether a table
> supports what it is about to do, but only InsertStmt, AlterTableStmt,
> DropTableOrViewStmt, LoadDataStmt, TruncateStmt and BaseTableRef go through
> it. MergeStmt, ModifyStmt and OptimizeStmt write to a table without ever
> asking.
> IMPALA-15310 needs such a check on those three, to refuse everything but
> INSERT INTO on an Iceberg table loaded from a REST catalog, and had to add a
> hand-written call to each of them rather than a case in
> checkTableCapability(). A new DML statement, or a new path into an existing
> one, now has to remember that call, and nothing points that out until the
> statement fails somewhere further down.
> Routing the three through checkTableCapability(WRITE) is the fix, and it is
> not a no-op: they would start running what ensureTableWriteSupported()
> applies, which is the bucketed-table check plus the HMS access-type check for
> everything except Kudu, or the transactional-table check on Metastore 2. That
> is a behaviour change for statements that have never run it, so it wants its
> own patch and its own testing rather than riding along with a REST catalog
> change.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
