You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by adrian-wang <gi...@git.apache.org> on 2014/12/11 12:28:01 UTC
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
GitHub user adrian-wang opened a pull request:
https://github.com/apache/spark/pull/3676
[SPARK-4829] [SQL] add rule to fold count(expr) if expr is not null
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/adrian-wang/spark countexpr
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3676.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3676
----
commit dc5765b1cf01553bcf2a24fee2b8447c951cd3ed
Author: Daoyuan Wang <da...@intel.com>
Date: 2014-12-11T08:57:05Z
add rule to fold count(expr) if expr is not null
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3676
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66613387
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24364/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66607006
[Test build #24364 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24364/consoleFull) for PR 3676 at commit [`dc5765b`](https://github.com/apache/spark/commit/dc5765b1cf01553bcf2a24fee2b8447c951cd3ed).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66613382
[Test build #24364 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24364/consoleFull) for PR 3676 at commit [`dc5765b`](https://github.com/apache/spark/commit/dc5765b1cf01553bcf2a24fee2b8447c951cd3ed).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66719303
This LGTM, but would like to share some findings related to semantics of `COUNT(expr)`. It seems that Hive has a bug here, and Spark SQL behaves differently from Hive.
The Hive language manual says [1] [1]:
> count(expr) - Returns the number of rows for which the supplied expression is non-NULL
but this doesn't conform to the following results (tested under Hive 0.13.1):
```sql
-- The test table `src1(key INT, value STRING)` is the one we used in Spark SQL `TestHiveContext`.
-- The table consists of 25 rows, among which 10 `key`s are `NULL`.
CREATE TABLE src1(key INT, value STRING);
LOAD DATA LOCAL INPATH 'data/files/kv3.txt' INTO TABLE src1;
SELECT COUNT(key) FROM src1
WHERE key IS NOT NULL; -- => 15, reasonable
SELECT COUNT(NULL) FROM src1; -- => 0, reasonable
SELECT COUNT(1) FROM src1; -- => 25, reasonable, 1 is never `NULL`
SELECT COUNT(key + 1) FROM src1; -- => 15, reasonable since `NULL + 1` is `NULL`.
SELECT COUNT(key) FROM src1; -- => 25, huh?
CREATE TABLE tmp AS
SELECT CAST(key AS STRING), value
FROM src1;
SELECT COUNT(key) FROM tmp; -- => 15, hm...
```
I'm not sure whether Hive has something equivalent to the `StructField.nullable` field in Spark SQL, but it seems that it always assumes `INT` as not nullable even if the underlying data may contain `NULL`. And `COUNT(expr)` doesn't check the actual data for null when `expr` is a single column whose data type is not nullable.
On the other hand, Spark SQL looks good. Here is a sample `hive/console` session:
```scala
scala> sql("SELECT COUNT(key) FROM src1").collect()
...
res2: Array[org.apache.spark.sql.Row] = Array([15]) // <- Reasonable
scala> table("src1").printSchema
root
|-- key: integer (nullable = true)
|-- value: string (nullable = true)
```
Notice that we consider all fields read from Hive Metastore nullable since data can be randomly dumped in without any validation.
[1]: https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by adrian-wang <gi...@git.apache.org>.
Github user adrian-wang commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66724763
Thanks for such detailed review! I checked your query with hive-0.14.0, the bug no longer exists.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66728650
Ah cool, couldn't open JIRA this morning to check whether it's a known issue :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-4829] [SQL] add rule to fold count(expr...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/3676#issuecomment-66738629
Thanks! Merged to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org