github-actions[bot] commented on code in PR #69363:
URL: https://github.com/apache/doris/pull/69363#discussion_r4335892976
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunction.java:
##########
@@ -336,17 +336,27 @@ public List<Column> getTableColumns() throws
AnalysisException {
TNetworkAddress address = new TNetworkAddress(be.getHost(),
be.getBrpcPort());
try {
- PFetchTableSchemaRequest request = getFetchTableStructureRequest();
+ if (getTFileType() == TFileType.FILE_STREAM) {
+ fileStatuses.add(new TBrokerFileStatus("", false, -1, true));
+ }
+ Iterator<TBrokerFileStatus> files = fileStatuses.iterator();
+ PFetchTableSchemaRequest request =
getFetchTableStructureRequest(files);
InternalService.PFetchTableSchemaResult result = null;
// `request == null` means we don't need to get schemas from BE,
// and we fill a dummy col for this table.
- if (request != null) {
+ while (request != null) {
Future<InternalService.PFetchTableSchemaResult> future =
BackendServiceProxy.getInstance()
.fetchTableStructureAsync(address, request);
result = future.get();
TStatusCode code =
TStatusCode.findByValue(result.getStatus().getStatusCode());
+ if (code == TStatusCode.NOT_FOUND) {
Review Comment:
[P2] Update the schema-failure test for NOT_FOUND retries
This retry changes the invocation count assumed by
`S3TableValuedFunctionTest.testSchemaFailureDoesNotSkipFiles`. Its NOT_FOUND
subcase supplies two nonempty files and returns NOT_FOUND for every schema RPC,
so the new loop calls `fetchTableStructureAsync` twice before throwing.
`assertSchemaFailure` still uses
`Mockito.verify(proxy).fetchTableStructureAsync(...)`, which requires exactly
one call, so that existing test now fails. Update its NOT_FOUND expectations
alongside the retry behavior, while retaining the single-call assertion for
other errors.
##########
fe/fe-core/src/test/java/org/apache/doris/job/offset/s3/S3EventSourceOffsetProviderTest.java:
##########
@@ -271,20 +304,65 @@ public void testSqsCloseFailureStillClosesFileSystem() {
}
@Test
- public void testNotificationMetadataDoesNotRequireHead() {
+ public void testNotificationUsesCurrentObjectMetadata() {
+ Mockito.when(s3Client.headObject(HeadObjectRequest.builder()
+ .bucket("bucket").key("logs/a,b[1].csv").build()))
+ .thenReturn(HeadObjectResponse.builder().contentLength(170L)
+
.lastModified(Instant.parse("2026-01-01T00:00:01Z")).eTag("new-etag").build());
Mockito.when(sqsClient.receiveMessage(Mockito.any(ReceiveMessageRequest.class)))
.thenReturn(response(message("literal", record("bucket",
"logs/a%2Cb%5B1%5D.csv", 123))));
provider.fetchRemoteMeta(jobProperties(1), tvfProperties);
S3EventOffset offset = nextOffset();
Assertions.assertEquals(Collections.singletonList("logs/a,b[1].csv"),
offset.getFiles());
Assertions.assertEquals("s3://bucket/logs/a,b[1].csv",
offset.getFileStatuses().get(0).getPath());
- Assertions.assertEquals(123,
offset.getFileStatuses().get(0).getSize());
+ Assertions.assertEquals(170,
offset.getFileStatuses().get(0).getSize());
Review Comment:
[P2] Reconcile the HEAD-based tests with the EVENT implementation
These tests cannot pass on this head:
`S3EventSourceOffsetProvider.parseMessage` never calls `headObject`, copies
`record.s3.object.size`, and assigns `System.currentTimeMillis()`. Here the
notification is 123 bytes, so this assertion receives 123 rather than 170. The
new missing-object, HEAD-failure, close-during-HEAD, mixed-record and both
resume cases likewise depend on unused HEAD stubs and fail their
assertions/verifications. Reconcile these tests with the intended
implementation before relying on this suite to validate overwrite, error and
close behavior.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
