Daniel Becker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21394 )

Change subject: IMPALA-13055: Some Iceberg metadata table tests don't assert
......................................................................


Patch Set 1:

(5 comments)

Thanks Gábor!

http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-13055: Some Iceberg metadata table tests don't assert
Do you think it is an error in the test framework? If so, could you open a 
separate Jira for it with repro instructions?


http://gerrit.cloudera.org:8080/#/c/21394/1//COMMIT_MSG@11
PS1, Line 11: unconditionally passes
We should make it clear the it is not the actual result from Impala that can be 
anything but the "expected" string.


http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test:

http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@49
PS1, Line 49: : VERIFY_IS_SUBSET
Is adding VERIFY_IS_SUBSET needed? I ran the test locally without it and it 
passed.


http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@365
PS1, Line 365: # Currently not supported, complex type slots are set to NULL 
(IMPALA-12205)
This comment is stale, we should change it.


http://gerrit.cloudera.org:8080/#/c/21394/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test@371
PS1, Line 371: '{.*}'
As this test section deals with complex types, I think we shouldn't put a 
wildcard for all the result. We should include details and use regexes for 
easily changing values (file size etc.).



--
To view, visit http://gerrit.cloudera.org:8080/21394
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie47093f25a70253b3e6faca27d466d7cf6999fad
Gerrit-Change-Number: 21394
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 May 2024 14:15:44 +0000
Gerrit-HasComments: Yes

Reply via email to