You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nsyca <gi...@git.apache.org> on 2016/07/29 21:56:49 UTC
[GitHub] spark pull request #14411: [SPARK-16804][SQL] Correlated subqueries containi...
GitHub user nsyca opened a pull request:
https://github.com/apache/spark/pull/14411
[SPARK-16804][SQL] Correlated subqueries containing LIMIT return incorrect results
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nsyca/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/14411.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 #14411
----
commit b98865127a39bde885f9b1680cfe608629d59d51
Author: Nattavut Sutyanyong <ns...@gmail.com>
Date: 2016-07-29T21:43:56Z
[SPARK-16804][SQL] Correlated subqueries containing LIMIT return incorrect results
## What changes were proposed in this pull request?
This patch fixes the incorrect results in the rule ResolveSubquery in Catalyst's Analysis phase.
## How was this patch tested?
./dev/run-tests
a new unit test on the problematic pattern.
commit 069ed8f8e5f14dca7a15701945d42fc27fe82f3c
Author: Nattavut Sutyanyong <ns...@gmail.com>
Date: 2016-07-29T21:50:02Z
[SPARK-16804][SQL] Correlated subqueries containing LIMIT return incorrect results
## What changes were proposed in this pull request?
This patch fixes the incorrect results in the rule ResolveSubquery in Catalyst's Analysis phase.
## How was this patch tested?
./dev/run-tests
a new unit test on the problematic pattern.
----
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3207 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3207/consoleFull)** for PR 14411 at commit [`631d396`](https://github.com/apache/spark/commit/631d396031e8bf627eb1f4872a4d3a17c144536c).
* This patch **fails Scala style 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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell ,
Yes. I agree that we need to block this case. I was under the impression that the tablesample clause is supported only when referenced to a base table, not a derived table. It's clearly not in Spark. I will add code to prevent it.
On the `NOT IN` topic, let me spend some time arranging my thought then I will open a JIRA.
thanks.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
We also need more test cases to prove it work as expected.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
@nsyca I think we do need to prevent sampling from being used. I have the following example:
```scala
range(0, 10).createOrReplaceTempView("tbl_a")
range(0, 10).select($"id", $"id" % 10 as "grp_id").createOrReplaceTempView("tbl_b")
range(0, 10).select($"id", $"id" % 10 as "grp_id").createOrReplaceTempView("tbl_c")
val plan = sql("""
select *
from tbl_a
where not exists(
select 1
from tbl_b
join (select *
from tbl_c
where tbl_c.id = tbl_a.id) tablesample(0.01 percent) c
on c.grp_id = tbl_b.grp_id)
""")
```
This results in the following analyzed plan:
```
Project [id#8L]
+- Filter NOT predicate-subquery#34 [(id#24L = id#8L)]
: +- SubqueryAlias predicate-subquery#34 [(id#24L = id#8L)]
: +- Project [1 AS 1#42, id#24L]
: +- Join Inner, (grp_id#29L = grp_id#19L)
: :- SubqueryAlias tbl_b
: : +- Project [id#14L, (id#14L % cast(10 as bigint)) AS grp_id#19L]
: : +- Range (0, 10, splits=8)
: +- SubqueryAlias c
: +- Sample 0.0, 1.0E-4, false, 968
: +- Project [id#24L, grp_id#29L]
: +- SubqueryAlias tbl_c
: +- Project [id#24L, (id#24L % cast(10 as bigint)) AS grp_id#29L]
: +- Range (0, 10, splits=8)
+- SubqueryAlias tbl_a
+- Range (0, 10, splits=8)
```
Clearly the predicate has been pulled out of a sampled relation. I don't think we want this.
I am looking forward to discuss your `NOT IN` approach. Could you open a JIRA for that?
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@gatorsmile: thanks. I will add them in SubquerySuite.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
Before you submitting the PR, you can run this command to check the scala style:
```dev/lint-scala```
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3199 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3199/consoleFull)** for PR 14411 at commit [`edca333`](https://github.com/apache/spark/commit/edca333c081e6d4e53a91b496fba4a3ef4ee89ac).
* This patch **fails Spark unit 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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
@nsyca No problem. We actually support `NOT IN` queries. We set the the `PredicateSubquery.nullAware` flag to true if we encounter an `IN` subquery expression. `NOT IN` is planned using a `NULL` aware anti-join in the optimizer.
I have written a blogpost/notebook on the current implementation and state of subqueries in Spark 2.0 (including known issues): [SQL Subqueries in Apache Spark 2.0](https://databricks.com/blog/2016/06/17/sql-subqueries-in-apache-spark-2-0.html)
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3206 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3206/consoleFull)** for PR 14411 at commit [`ac43ab4`](https://github.com/apache/spark/commit/ac43ab47907a1ccd6d22f920415fbb4de93d4720).
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3206 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3206/consoleFull)** for PR 14411 at commit [`ac43ab4`](https://github.com/apache/spark/commit/ac43ab47907a1ccd6d22f920415fbb4de93d4720).
* This patch **fails Scala style 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 #14411: [SPARK-16804][SQL] Correlated subqueries containi...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/14411#discussion_r73746412
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -1021,6 +1021,19 @@ class Analyzer(
case e: Expand =>
failOnOuterReferenceInSubTree(e, "an EXPAND")
e
+ case l @ LocalLimit(_, _) =>
--- End diff --
Style: use `l: LocalLimit` instead of `l @ LocalLimit(_, _)` it makes it a bit more readable. Same for `GlobalLimit` and `Sample`.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
Ok, this looks pretty good. One overall comment: We are basically blacklisting operators here (which is fine IMO), should we check if there any other operators we should about? If there are we might need to generalize the blacklisting (instead of working case by case)
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
@nsyca We do not rewrite the subquery into a join during analysis. We rewrite subqueries into joins during optimization. We do two things during analysis:
1. We check if the subquery expression is valid. In order to do this we need to check if the query resolves (given the outer query), and that no outer references are used in (the children of) nodes for which the joining behavior is ill defined (`UNION` for instance).
2. We also rewrite IN/EXISTS/Scalar subquery expressions into a PredicateSubquery. We do this by extracting correlated predicates and by rewriting the intermediate tree. One could argue that this could also be done during optimization, but this was needed to get correlated predicates with aggregate functions (referencing the outer query) working (see the example below). For this we needed to push the complete outer condition into the Aggregate below the Having clause. Perhaps there is a simpler way of doing this though.
```SQL
select b.key, min(b.value)
from src b
group by b.key
having exists ( select a.key
from src a
where a.value > 'val_9' and a.value = min(b.value)
)
```
I think we should also limit the use of `Sample`, which also filters non-deterministically and might give us very wrong results as well.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell,
Thanks for getting the PR merged and sorry for causing a few hiccups before I got it right. It's my first PR.
I have opened a new JIRA, SPARK-16951, to track the NOT IN issue. Currently it still contains little information but I will start to fill in in the next few days.
Btw, would you mind assigning me (nsyca) the Assignee of SPARK-16804?
Will look forward to collaborating with you in future issues.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
Merging to master. Thanks for working on this!
Ping me as soon as you open a JIRA for null aware anti joins.
---
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 #14411: [SPARK-16804][SQL] Correlated subqueries containi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/14411
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14411
Can one of the admins verify this patch?
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
It sounds great to me!
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3208 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3208/consoleFull)** for PR 14411 at commit [`7eb9b2d`](https://github.com/apache/spark/commit/7eb9b2dbba3633a1958e38e0019e3ce816300514).
* 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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
I think the positive cases can move here.
`sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala`
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
@nsyca I have triggered a manual build. I'll merge as soon as it completes successfully.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell,
Code and test case for blocking `TABLESAMPLE` is in. Could you please review? Thanks.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
Thanks, @gatorsmile. This time I ran `dev/lint-scala`. Hope it's my last attempt to get this work thru.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
retest this please
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
Done.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3207 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3207/consoleFull)** for PR 14411 at commit [`631d396`](https://github.com/apache/spark/commit/631d396031e8bf627eb1f4872a4d3a17c144536c).
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3199 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3199/consoleFull)** for PR 14411 at commit [`edca333`](https://github.com/apache/spark/commit/edca333c081e6d4e53a91b496fba4a3ef4ee89ac).
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3200 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3200/consoleFull)** for PR 14411 at commit [`64184fd`](https://github.com/apache/spark/commit/64184fdb77c1a305bb2932e82582da28bb4c0e53).
* 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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell,
Have you had a chance to review my last update? Are there anything I should add/change in this PR?
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell , thanks for sharing the blog. I will read thru. It's nice to see the implementation of NOT IN this way. I have an idea to do it differently but let's move this to another place.
On the SAMPLE issue you raised, I think we should not flag an error. Here is what I tested:
Seq((1,1), (2,2)).toDF("c1","c2").createOrReplaceTempView("t1")
Seq((1,1), (2,2)).toDF("c3","c4").createOrReplaceTempView("t2")
scala> sql("select * from t1 where exists (select 1 from t2 tablesample(10 percent) s where c3=c1)").explain(true)
== Parsed Logical Plan ==
'Project [*]
+- 'Filter exists#29
: +- 'SubqueryAlias exists#29
: +- 'Project [unresolvedalias(1, None)]
: +- 'Filter ('c3 = 'c1)
: +- 'Sample 0.0, 0.1, false, 159
: +- 'UnresolvedRelation `t2`, s
+- 'UnresolvedRelation `t1`
From the parser, the correlated predicate in the Filter operation is after the sampling operation. We should be able to treat the semantic of the sampling as an one-time execution and being reused for every input from the outer table. Using the analogy I used for LIMIT as described in the JIRA SPARK=16804, the SAMPLE operation is not on the correlation path and therefore the move of the correlated predicate above the scope of the subquery does not change the semantic of the query.
Your thoughts, please!
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14411
LGTM - pending Jenkins.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
cc @hvanhovell
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
I include two examples of "good" case in the JIRA to show that this fix only blocks cases where Spark will produce incorrect results. I need to find a place to host those "good" cases. Don't think AnalysisErrorSuite.scala is the right place.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell,
First, my apologies for delaying the replies. I am travelling this week, only getting spontaneous connections. Thank you for your explanation of the implementation and the reason behind the choice of the implementation. It is very helpful for a beginner like me.
My bad! What I meant in my previous comment on rewriting of subqueries to join is actually the moving of the positions of the correlated predicates from their original positions to outside of the scopes of subqueries, specifically, the call to the function pullOutCorrelatedPredicates() -- I hope I got it right this time. I see this as one of the root causes of many problems. Bear with me, I don't have a good solution as I am still getting myself familiar with the code. Here is an example of the problems, in my opinion. With the rewrite, we cannot distinct between the EXISTS form and IN form of the original SQL.
select * from t1 where exists (select 1 from t2 where t1.c1=t2.c2)
-and-
select * from t1 where t1.c1 in (select t2.c2 from t2)
are represented after Analysis phase. This does not have issue because they are semantically equivalent. However, when we add the NOT in
select * from t1 where not exists (select 1 from t2 where t1.c1=t2.c2)
-and-
select * from t1 where t1.c1 not in (select t2.c2 from t2)
are NOT semantically equivalent when T2.C2 can produce NULL values.
Lastly, your comment on the operator SAMPLE seems right. I will give it shot on adding it to this PR.
Thanks again for your patience.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
@hvanhovell,
Thank you for your comment. There are quite a few patterns being blacklisted already, such as correlation under set operators (UNION, EXCEPT, INTERSECT), correlation outside of WHERE/HAVING context, correlation in the right table of a LEFT [OUTER] JOIN (and the left table of a RIGHT [OUTER] JOIN]). I am working on discovering more issues in this area but it looks like a bigger project to me. I have a general idea that the rewrite of correlation subquery to join should not happen in the Analysis phase. We should build a Logical plan to represent the subquery and perform the rewrite at the Optimizer phase instead.
I am new to the Spark code and this is my first PR. So I'd like to make it a small, self-contained project to gain my confidence in working with the code.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing non-...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3208 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3208/consoleFull)** for PR 14411 at commit [`7eb9b2d`](https://github.com/apache/spark/commit/7eb9b2dbba3633a1958e38e0019e3ce816300514).
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14411
**[Test build #3200 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3200/consoleFull)** for PR 14411 at commit [`64184fd`](https://github.com/apache/spark/commit/64184fdb77c1a305bb2932e82582da28bb4c0e53).
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by nsyca <gi...@git.apache.org>.
Github user nsyca commented on the issue:
https://github.com/apache/spark/pull/14411
Two good cases, which return the same result set, with and without this proposed fix:
sql("select c1 from t1 where exists (select 1 from t2 where t1.c1=t2.c2) and exists (select 1 from t2 LIMIT 1)").show
The above query will return both rows from T1.
sql("select c1 from t1 where exists (select 1 from (select 1 from t2 limit 1) where t1.c1=t2.c2)").show
This one above will return 1 row but which row will return is non-deterministic depending on what the first row from T2 will return from the innermost subquery.
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
@nsyca Could you update the PR description? Thanks!
---
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 issue #14411: [SPARK-16804][SQL] Correlated subqueries containing LIMI...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/14411
retest this please
---
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