jerryshao commented on code in PR #12551:
URL: https://github.com/apache/gravitino/pull/12551#discussion_r3853148237
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java:
##########
@@ -139,15 +131,34 @@ public void insertTable(TableEntity tableEntity, boolean
overwrite) throws IOExc
SessionUtils.doWithoutCommit(
TableMetaMapper.class,
mapper -> {
- tablePORef.set(po);
ops.insertPO(mapper, po, overwrite);
+ if (overwrite) {
+ // MySQL may resolve the upsert through the active
(schema_id, table_name,
+ // deleted_at) key rather than table_id. In that case it
preserves the
+ // winner's ID. The upsert already holds that row until
commit, so read the
+ // database-derived identity and version back through
the same natural key.
+ TablePO storedPO =
+ mapper.selectTableMetaBySchemaIdAndName(
Review Comment:
**Efficiency/simplification: unconditional extra round trip + unexplained
guard on the overwrite path.**
Two related points in this overwrite branch of `insertTable`:
1. `selectTableMetaBySchemaIdAndName` now runs on *every* `insertTable(...,
overwrite=true)` call, not only when the natural key actually collided with a
different table_id. It's needed because `current_version`/`last_version` are
now computed inside the UPDATE (`current_version + 1`) rather than passed in,
so Java has to read the result back — this adds a fixed extra round trip to the
common overwrite path that didn't exist before this PR (previously
`insertTableVersionOnDuplicateKeyUpdate(po)` used the pre-computed `po`
directly).
2. The `storedPO.getCurrentVersion() > POConverters.INIT_VERSION` guard a
few lines below (before soft-deleting the prior version row) isn't explained by
a comment — it reads as if it exists to avoid an unnecessary no-op delete for
the "brand new row via INSERT branch" case, but that's worth
confirming/documenting explicitly, since a reader has to reconstruct why the
guard is only sometimes needed.
--
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]
