You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by viirya <gi...@git.apache.org> on 2015/12/24 10:32:19 UTC
[GitHub] spark pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
GitHub user viirya opened a pull request:
https://github.com/apache/spark/pull/10470
[SPARK-12409][SPARK-12387][SPARK-12391][SQL] Refactor filter pushdown for JDBCRDD and add few filters
This patch refactors the filter pushdown for JDBCRDD and also adds few filters.
Added filters are basically from #10468 with some refactoring. Test cases are from #10468.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/viirya/spark-1 refactor-jdbc-filter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10470.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 #10470
----
commit 1e3adbc728b03b50850d461da6d4d7e7d00b8e37
Author: Liang-Chi Hsieh <vi...@gmail.com>
Date: 2015-12-24T09:27:27Z
Refactor filter pushdown for JDBCRDD and add few filters.
----
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167093975
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48302/
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167079820
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 pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167932027
@rxin Okay and I did.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167934264
**[Test build #48449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48449/consoleFull)** for PR 10470 at commit [`a7ef79e`](https://github.com/apache/spark/commit/a7ef79e7b51aeac2d0cae47914654dc42a64492a).
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48525943
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -184,16 +185,38 @@ private[sql] object JDBCRDD extends Logging {
* Turns a single Filter into a String representing a SQL expression.
* Returns null for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
+ if (or.size == 2) {
--- End diff --
Does this work? I mean. It looks the `or.size` is always `2` due to `null`. `flatten` does not filter `null`s.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168290999
ping @rxin
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168121319
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48534/
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168112759
**[Test build #48534 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48534/consoleFull)** for PR 10470 at commit [`cb7ce21`](https://github.com/apache/spark/commit/cb7ce21150e08e12e5eca3e5d08cfa4f2a105adc).
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48585579
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
Do we already support all filters here?
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167761905
Merged build finished. 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167761907
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48408/
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167096777
Merged build finished. 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168121072
**[Test build #48534 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48534/consoleFull)** for PR 10470 at commit [`cb7ce21`](https://github.com/apache/spark/commit/cb7ce21150e08e12e5eca3e5d08cfa4f2a105adc).
* 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167943198
Merged build finished. 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586148
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -295,25 +318,17 @@ private[sql] class JDBCRDD(
/**
* `filters`, but as a WHERE clause suitable for injection into a SQL query.
*/
- private val filterWhereClause: String = {
- val filterStrings = filters.map(JDBCRDD.compileFilter).filter(_ != null)
- if (filterStrings.size > 0) {
- val sb = new StringBuilder("WHERE ")
- filterStrings.foreach(x => sb.append(x).append(" AND "))
- sb.substring(0, sb.length - 5)
- } else ""
- }
+ private val filterWhereClause: Seq[Option[String]] = filters.map(JDBCRDD.compileFilter)
--- End diff --
yes. I added flatten to it and return Array[String] then.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167761842
**[Test build #48408 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48408/consoleFull)** for PR 10470 at commit [`b0da6fc`](https://github.com/apache/spark/commit/b0da6fc402ad001f1a8a20dcf882fedb880b3f75).
* 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586143
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -295,25 +318,17 @@ private[sql] class JDBCRDD(
/**
* `filters`, but as a WHERE clause suitable for injection into a SQL query.
*/
- private val filterWhereClause: String = {
- val filterStrings = filters.map(JDBCRDD.compileFilter).filter(_ != null)
- if (filterStrings.size > 0) {
- val sb = new StringBuilder("WHERE ")
- filterStrings.foreach(x => sb.append(x).append(" AND "))
- sb.substring(0, sb.length - 5)
- } else ""
- }
+ private val filterWhereClause: Seq[Option[String]] = filters.map(JDBCRDD.compileFilter)
/**
* A WHERE clause representing both `filters`, if any, and the current partition.
*/
private def getWhereClause(part: JDBCPartition): String = {
- if (part.whereClause != null && filterWhereClause.length > 0) {
- filterWhereClause + " AND " + part.whereClause
- } else if (part.whereClause != null) {
- "WHERE " + part.whereClause
+ val w = filterWhereClause.+:(Option(part.whereClause)).flatten.mkString(" AND ")
--- End diff --
ok. I revert it back to old code with little difference for the new filterWhereClause.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167079490
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48301/
Test FAILed.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167750063
**[Test build #48408 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48408/consoleFull)** for PR 10470 at commit [`b0da6fc`](https://github.com/apache/spark/commit/b0da6fc402ad001f1a8a20dcf882fedb880b3f75).
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48527331
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -184,16 +185,38 @@ private[sql] object JDBCRDD extends Logging {
* Turns a single Filter into a String representing a SQL expression.
* Returns null for an unhandled filter.
--- End diff --
ok.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167096711
**[Test build #48303 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48303/consoleFull)** for PR 10470 at commit [`05188b7`](https://github.com/apache/spark/commit/05188b73d3b80023749cf281f0ede194108baa33).
* 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48566796
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -295,25 +318,17 @@ private[sql] class JDBCRDD(
/**
* `filters`, but as a WHERE clause suitable for injection into a SQL query.
*/
- private val filterWhereClause: String = {
- val filterStrings = filters.map(JDBCRDD.compileFilter).filter(_ != null)
- if (filterStrings.size > 0) {
- val sb = new StringBuilder("WHERE ")
- filterStrings.foreach(x => sb.append(x).append(" AND "))
- sb.substring(0, sb.length - 5)
- } else ""
- }
+ private val filterWhereClause: Seq[Option[String]] = filters.map(JDBCRDD.compileFilter)
--- End diff --
should this just take Seq[String] by adding a flatMap to filters.map(JDBCRDD.compileFilter)?
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48588506
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
Good to know 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 pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168121315
Merged build finished. 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48566823
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
@@ -186,8 +187,26 @@ class JDBCSuite extends SparkFunSuite
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME = 'fred'")).collect().size == 1)
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME > 'fred'")).collect().size == 2)
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME != 'fred'")).collect().size == 2)
+ assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME IN ('mary', 'fred')"))
+ .collect().size == 2)
+ assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME NOT IN ('fred')"))
--- End diff --
use two == to be consistent
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167079488
Merged build finished. Test FAILed.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586845
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
Oh actually I opened a PR for `EqualNullSafe` here already few months ago. https://github.com/apache/spark/pull/8743/files
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168294357
Thanks - I've merged this.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168078143
@viirya I've merged https://github.com/apache/spark/pull/10468 now.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10470
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167093974
Merged build finished. 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167078592
**[Test build #48302 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48302/consoleFull)** for PR 10470 at commit [`05188b7`](https://github.com/apache/spark/commit/05188b73d3b80023749cf281f0ede194108baa33).
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48566768
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -295,25 +318,17 @@ private[sql] class JDBCRDD(
/**
* `filters`, but as a WHERE clause suitable for injection into a SQL query.
*/
- private val filterWhereClause: String = {
- val filterStrings = filters.map(JDBCRDD.compileFilter).filter(_ != null)
- if (filterStrings.size > 0) {
- val sb = new StringBuilder("WHERE ")
- filterStrings.foreach(x => sb.append(x).append(" AND "))
- sb.substring(0, sb.length - 5)
- } else ""
- }
+ private val filterWhereClause: Seq[Option[String]] = filters.map(JDBCRDD.compileFilter)
/**
* A WHERE clause representing both `filters`, if any, and the current partition.
*/
private def getWhereClause(part: JDBCPartition): String = {
- if (part.whereClause != null && filterWhereClause.length > 0) {
- filterWhereClause + " AND " + part.whereClause
- } else if (part.whereClause != null) {
- "WHERE " + part.whereClause
+ val w = filterWhereClause.+:(Option(part.whereClause)).flatten.mkString(" AND ")
--- End diff --
the old code here is a lot easier to understand than the new version.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167081200
**[Test build #48303 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48303/consoleFull)** for PR 10470 at commit [`05188b7`](https://github.com/apache/spark/commit/05188b73d3b80023749cf281f0ede194108baa33).
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48585459
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
Please add tests in case of failing to compile sub-filters.
What is an concrete example 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 pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48585283
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -295,25 +318,17 @@ private[sql] class JDBCRDD(
/**
* `filters`, but as a WHERE clause suitable for injection into a SQL query.
*/
- private val filterWhereClause: String = {
- val filterStrings = filters.map(JDBCRDD.compileFilter).filter(_ != null)
- if (filterStrings.size > 0) {
- val sb = new StringBuilder("WHERE ")
- filterStrings.foreach(x => sb.append(x).append(" AND "))
- sb.substring(0, sb.length - 5)
- } else ""
- }
+ private val filterWhereClause: Seq[Option[String]] = filters.map(JDBCRDD.compileFilter)
/**
* A WHERE clause representing both `filters`, if any, and the current partition.
*/
private def getWhereClause(part: JDBCPartition): String = {
- if (part.whereClause != null && filterWhereClause.length > 0) {
- filterWhereClause + " AND " + part.whereClause
- } else if (part.whereClause != null) {
- "WHERE " + part.whereClause
+ val w = filterWhereClause.+:(Option(part.whereClause)).flatten.mkString(" AND ")
--- End diff --
+1 and the old one is better 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 pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48524499
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -184,16 +185,38 @@ private[sql] object JDBCRDD extends Logging {
* Turns a single Filter into a String representing a SQL expression.
* Returns null for an unhandled filter.
--- End diff --
need to update this comment
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586131
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
I added a test for that using EqualNullSafe.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586155
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
@@ -186,8 +187,26 @@ class JDBCSuite extends SparkFunSuite
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME = 'fred'")).collect().size == 1)
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME > 'fred'")).collect().size == 2)
assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME != 'fred'")).collect().size == 2)
+ assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME IN ('mary', 'fred')"))
+ .collect().size == 2)
+ assert(stripSparkFilter(sql("SELECT * FROM foobar WHERE NAME NOT IN ('fred')"))
--- End diff --
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 pull request: [SPARK-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167738141
@maropu can you review this change?
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586488
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
My bad, and I understood.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167943160
**[Test build #48449 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48449/consoleFull)** for PR 10470 at commit [`a7ef79e`](https://github.com/apache/spark/commit/a7ef79e7b51aeac2d0cae47914654dc42a64492a).
* 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167093907
**[Test build #48302 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48302/consoleFull)** for PR 10470 at commit [`05188b7`](https://github.com/apache/spark/commit/05188b73d3b80023749cf281f0ede194108baa33).
* 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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167096779
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48303/
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-168112310
@rxin Rebased. Wait for passing tests.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48588333
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
Oh great. If #8743 merged, I think it could simplify the `And`/`Or` entries in `compileFilters`.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48585861
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
I think that this pr includes all the implemented filters in `o.a.s.sources.filters`.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/10470#discussion_r48586764
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
@@ -182,18 +183,40 @@ private[sql] object JDBCRDD extends Logging {
/**
* Turns a single Filter into a String representing a SQL expression.
- * Returns null for an unhandled filter.
+ * Returns None for an unhandled filter.
*/
- private def compileFilter(f: Filter): String = f match {
- case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
- case Not(EqualTo(attr, value)) => s"$attr != ${compileValue(value)}"
- case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
- case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
- case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
- case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
- case IsNull(attr) => s"$attr IS NULL"
- case IsNotNull(attr) => s"$attr IS NOT NULL"
- case _ => null
+ private def compileFilter(f: Filter): Option[String] = {
+ Option(f match {
+ case EqualTo(attr, value) => s"$attr = ${compileValue(value)}"
+ case LessThan(attr, value) => s"$attr < ${compileValue(value)}"
+ case GreaterThan(attr, value) => s"$attr > ${compileValue(value)}"
+ case LessThanOrEqual(attr, value) => s"$attr <= ${compileValue(value)}"
+ case GreaterThanOrEqual(attr, value) => s"$attr >= ${compileValue(value)}"
+ case IsNull(attr) => s"$attr IS NULL"
+ case IsNotNull(attr) => s"$attr IS NOT NULL"
+ case StringStartsWith(attr, value) => s"${attr} LIKE '${value}%'"
+ case StringEndsWith(attr, value) => s"${attr} LIKE '%${value}'"
+ case StringContains(attr, value) => s"${attr} LIKE '%${value}%'"
+ case In(attr, value) => s"$attr IN (${compileValue(value)})"
+ case Not(f) => compileFilter(f).map(p => s"(NOT ($p))").getOrElse(null)
+ case Or(f1, f2) =>
+ // We can't compile Or filter unless both sub-filters are compiled successfully.
+ // It applies too for the following And filter.
+ val or = Seq(f1, f2).map(compileFilter(_)).flatten
--- End diff --
My bad and I understood.
Actually, I'm not sure how to handle `EqualNullSafe`. Most the recent databases support `IS NOT DISTINCT FROM` corresponding to `EqualNullSafe` in SQL99 though, older databases do not.
ISTM that following prs consider this.
---
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-12409][SPARK-12387][SPARK-12391][SQL] R...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10470#issuecomment-167943199
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48449/
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