skywalker0618 opened a new pull request, #20145:
URL: https://github.com/apache/hudi/pull/20145
### Describe the issue this Pull Request addresses
In `hive_sync.mode=hiveql`, a statement Hive rejects does not fail the sync.
Hive's
`Driver.run(sql)` reports compile and execution failures (a partition spec
naming a column the
table is not partitioned by, a `REPLACE COLUMNS` the metastore refuses, a
missing database) through
a non-zero `CommandProcessorResponse` rather than by throwing.
`HiveQueryDDLExecutor` collected
those responses and never looked at them, and `HiveDriverPool` discarded
them outright, so the only
trace of the failure was Hive's own `ERROR org.apache.hadoop.hive.ql.Driver
- FAILED: ...` log line.
The sync then carried on as if the DDL had been applied:
- it reported success,
- it advanced `last_commit_time_sync` past the commit whose partitions were
never registered, so a
later incremental sync does not retry them,
- and `hoodie.datasource.hive_sync.recreate_table_on_error` never fired in
HiveQL mode, since it
only reacts to `HoodieHiveSyncException`.
The JDBC and HMS modes already fail on `SQLException` and `TException`
respectively; HiveQL mode
was the odd one out.
Observed on a Flink job whose table had been registered in the metastore
partitioned by
`_hoodie_partition_path`, while the job synced with
`partition_fields=datestr`. Every cycle logged
`Partition spec {datestr=...} contains non-partition columns` from Hive, and
every cycle reported a
successful sync.
### Summary and Changelog
HiveQL-mode sync now fails with `HoodieHiveSyncException` when Hive rejects
a statement, carrying
Hive's error message, the SQLState and the statement (abbreviated past 1,000
characters, since
batched partition statements can run to many kilobytes).
- New `HiveDriverUtil.runOrThrow(Driver, String)` runs the statement and
checks the response code.
Both execution paths use it: the sequential path in
`HiveQueryDDLExecutor.updateHiveSQLs`, and
both `HiveDriverPool.runOnEachWorker` (the per-worker `USE` statements) and
`HiveDriverPool.dispatchAll`.
- On the sequential path the sync stops at the first rejected statement, and
the exception is
rethrown as-is rather than re-wrapped as a generic "Failed in executing
SQL". The pool path
reports it through `awaitAll`'s existing first-error handling, which
already cancels the
statements queued behind a failure.
- `updateHiveSQLs` no longer returns the response list, which had no callers.
- Tests: `TestHiveSyncTool#testHiveQLSyncFailsWhenHiveRejectsAStatement`
reproduces the case above
against the embedded metastore, with batching both off and on. It
registers the table partitioned
by `_hoodie_partition_path`, then syncs a new commit with
`partition_fields=datestr`, and asserts
that the sync fails with Hive's rejection and `last_commit_time_sync`
stays at the earlier
commit. Unit tests in `TestHiveDriverPool` and
`TestHiveQueryDDLExecutorSession` cover each
path, including that the thread's session and class loader are still
handed back after a
rejected statement. Existing mocks that returned `null` from `Driver.run`
(which a real Driver
never does) now return a success response.
### Impact
No public API or config change. Behaviour change: a HiveQL-mode sync whose
DDL Hive rejects now
fails instead of silently reporting success. For Spark writers this surfaces
as the existing
`HoodieMetaSyncException` after the commit has completed; for Flink it is
logged by the
coordinator's sync executor. `recreate_table_on_error` now takes effect in
HiveQL mode as it
already did in the other modes. Tables whose metastore definition had
drifted from the sync
configuration will start reporting it, which is the intent.
### Risk Level
low
The statements the sync issues are idempotent (`CREATE DATABASE IF NOT
EXISTS`,
`ADD IF NOT EXISTS PARTITION`, `REPLACE COLUMNS`, `SET LOCATION`, `TOUCH`),
so a healthy table does
not produce non-zero responses. The full `hudi-hive-sync` suite passes (404
tests, including all
277 in `TestHiveSyncTool`, which runs the HiveQL path end-to-end). All five
new tests fail with the
production change reverted.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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]
