Takch02 commented on PR #4137:
URL: https://github.com/apache/amoro/pull/4137#issuecomment-4224375098
@xxubai Thank you for the feedback! I agree that partial Hive visibility
should not be treated as valid behavior.
However, after investigating, I found that the current production code does
not throw `CannotAlterHiveLocationException` for non-partitioned tables — it
simply commits successfully even when files span multiple directories. So if I
update the non-partitioned tests to expect the exception, they will fail
against the current implementation.
To fully address this, the production logic (likely in `UpdateHiveFiles`)
would also need to be updated to enforce the same directory constraint for
non-partitioned tables.
Would it be okay to include that production fix in this PR, or would you
prefer to handle it in a separate PR?
```
@Test
public void testRewritePartFilesNonPartitioned() {
Assume.assumeFalse(isPartitionedTable());
...
List<DataFile> dataFiles =
HiveDataTestHelpers.writerOf(getMixedTable()).transactionId(1L).writeHive(insertRecords);
RewriteFiles rewriteFiles = baseStore.newRewrite();
rewriteFiles.rewriteFiles(Sets.newHashSet(deleteFile),
Sets.newHashSet(dataFiles));
Assert.assertThrows(CannotAlterHiveLocationException.class,
rewriteFiles::commit); // False
}
--
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]
