Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21568
thanks, merging to master!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21568
LGTM
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92857/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21568
**[Test build #92857 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92857/testReport)**
for PR 21568 at commit
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
@cloud-fan @rxin I updated the PR in order to handle only the case in which
we have the same result for different configs and the configs are specified in
the SQL files.
In order to make clear
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21568
**[Test build #92857 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92857/testReport)**
for PR 21568 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/847/
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
To me it is actually confusing to have the decimal one in there at all, by
defining a list of queries that are reused for different functional
testing. It is very easy to just ignore the subtle
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21568
We can deal with the decimal test file specially if that's the only use
case. For now I'd say the join test is more important and let's finish it first.
---
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
I am not sure it is a great idea to do only one of the 2 scenarios, if we
plan to later include both them as we might have to redo the same work twice.
But if you all agree on this plan I'll stick
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21568
It's good to cover both of the cases in one design, but I'd like to
prioritize the join one.
I feel it's common to try with different optimization/runtime configs and
make sure we get
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
The main other case here was to improve also the coverage for the join
operations, because with the default values we are testing only the broadcast
join.
---
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
What are the use cases other than decimal? I am not sure if we need to
build a lot of infrastructure just for one or two use cases.
---
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
The goal here was to address both cases. The need came out in previous PRs
in order to avoid to copy and paste the same queries in order to test them with
different configs. So the idea here was
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
If they produce different results why do you need any infrastructure for
them? They are just part of the normal test flow.
If they produce the same result, and you don't want to define the
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
Sorry,what do you mean by a config matrix? And how would we discriminate
whether each config should produce the same result or they should produce
different ones?
---
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
Can you just define a config matrix in the beginning of the file, and each
file is run with the config matrix?
---
-
To
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
@rxin then we can do what @maropu suggested, i.e. adding a numeric suffix
and maybe logging the used configs so even though we.don't have them in the
test name we can anyway know which of them is
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
I think it's super confusing to have the config names encoded in file
names. Makes the names super long and difficult to read, and also hard to
verify what was set, and difficult to get multiple
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
kindly ping @cloud-fan
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
@maropu I think we can, instead. Since the configs are put as suffix in the
test case name (this happens also in the same result case) you know which
configs failed.
---
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21568
In the same result case, I'm worried that we cannot easily understand which
SQL configs cause failures?
IMHO `withSQLConf` has the same issue, too;
```
Seq(true, false).foreach { flag =>
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
Sorry, @maropu, I didn't get want you mean by
> If we accepted the same output files for that case
may you please explain me?
Anyway, the problem with that proposal is not
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21568
We need to strictly handle the second case, too? If we accepted the same
output files for that case, we could have simpler output file name rules as I
described
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
@rxin the PR does what was suggested in these 2 comments
https://github.com/apache/spark/pull/20023#issuecomment-358306751 and
https://github.com/apache/spark/pull/21529#issuecomment-396413144.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21568
IIUC this pr modified code to run a single test file multiple times in
`SQLQueryTestSuite` with different configurations.
---
-
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/21568
I'm confused by the description. What does this PR actually do?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91859/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21568
**[Test build #91859 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91859/testReport)**
for PR 21568 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/4040/
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21568
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/149/
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21568
**[Test build #91859 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91859/testReport)**
for PR 21568 at commit
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/21568
cc @cloud-fan
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
38 matches
Mail list logo
