impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Wood (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
Date Fri, 22 Sep 2017 18:13:59 GMT
Tim Wood has posted comments on this change. ( http://gerrit.cloudera.org:8080/8102 )

Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for Impala.
......................................................................


Patch Set 3:

(8 comments)

Some comments from PS 1 got erased in PS 2 because their files were renamed & replaced.

PS 2 and PS 3 only differ in commit msg.

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

http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@11
PS1, Line 11: Data set constructed in mini-cluster using $IMPALA_HOME/bin/buildall.sh -testdata....
> I think this sentence is awkward. "incubator-impala" happens to be what the
Done


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@22
PS1, Line 22: y don't r
> > if the test data (generator) is public as well, then we should replace th
Done - This set of commits is completely free-standing in the ASF repo.  Verified from build
results on jenkins.impala.io.


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@22
PS1, Line 22: y don't r
> This looks like internal Cloudera information. Please excise all internal C
Done


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@22
PS1, Line 22: y don't r
> CDH-xxx94 replaced with IMPALA-5960.
Done


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@22
PS1, Line 22: y don't r
> I logged a CDH ticket here because the issue is with the test data.  But if
Done


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@25
PS1, Line 25: The following list describes the current status as:
            : --- test-name
            : deviance from TPC-DS spec
            : changes made
> These are good comments, but they are not line-wrapped. They could also sta
Done


http://gerrit.cloudera.org:8080/#/c/8102/1//COMMIT_MSG@32
PS1, Line 32: Fixed AVG()s
> Comments like this are useful. It would be good for you to note this in tpc
Done


http://gerrit.cloudera.org:8080/#/c/8102/1/tests/query_test/test_tpcds_queries.py
File tests/query_test/test_tpcds_queries.py:

http://gerrit.cloudera.org:8080/#/c/8102/1/tests/query_test/test_tpcds_queries.py@87
PS1, Line 87:   @pytest.mark.xfail(reason="IMPALA-5226")
            :   def test_tpcds_q10(self, vector):
            :     self.run_test_case(self.get_workload() + '-q10', vector)
> While this works for functional end-to-end tests, we need a way to tell the
Discussed with Matt.  I split up all former multi-query tests into individual files.  As the
stress test was exposed to skip/xfail cases previous to this work, the status-quo handling
of these should be sufficient.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66
Gerrit-Change-Number: 8102
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wood <twood@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mmulder@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Tim Wood <twood@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Sep 2017 18:13:59 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message