yuqi1129 commented on code in PR #12551:
URL: https://github.com/apache/gravitino/pull/12551#discussion_r3854040455
##########
core/src/main/java/org/apache/gravitino/storage/relational/po/TablePO.java:
##########
@@ -120,6 +120,28 @@ private Builder() {
tablePO = new TablePO();
}
+ // Copies every column of the source row. When a field is added to TablePO
above, add it here
+ // too, otherwise callers that copy a row would silently blank it.
+ private Builder(TablePO source) {
Review Comment:
The hand-written copy is still there, but it is no longer guarded only by a
comment: `TestTablePO#testCopyBuilderCarriesEveryField` compares every declared
field of source and copy reflectively, and the fixture asserts each field was
set, so a field added later without a matching line in the builder fails the
test rather than silently blanking on copy.
Removing the hand-written copy altogether would mean changing how `TablePO`
is written, which I would rather not do inside this PR.
##########
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:
Added a comment for the guard: there is an earlier version row to retire
only when the upsert updated an existing table, which moves the version from N
to N+1; a fresh insert leaves it at the initial version with no earlier row.
On the extra round trip, I could not find a way to avoid it. The version is
now derived inside the statement (`current_version + 1`), so the resulting
version has to be read back before `table_version_info` can be keyed by it.
Deriving it with a subquery would need three different spellings for MySQL,
PostgreSQL and H2. The read itself is a single-table select, cheaper than
`selectTableMetaById`, which joins `table_version_info`. Happy to revisit if
you see a portable way to fold it in.
--
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]
