You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by SharpRay <gi...@git.apache.org> on 2017/06/28 08:55:41 UTC
[GitHub] spark pull request #18451: [SPARK-18004][SQL] Make sure the date or timestam...
GitHub user SharpRay opened a pull request:
https://github.com/apache/spark/pull/18451
[SPARK-18004][SQL] Make sure the date or timestamp related predicate can be pushed down to Oracle correctly
## What changes were proposed in this pull request?
Move `compileValue` method in JDBCRDD to JdbcDialect, and override the `compileValue` method in OracleDialect to rewrite the Oracle-specific timestamp and date literals in where clause.
## How was this patch tested?
An integration test has been added.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SharpRay/spark extend-compileValue-to-dialects
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/18451.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 #18451
----
commit 770bf9c80a3f0411d97fa397b2e162af76d9a022
Author: Rui Zha <zr...@gmail.com>
Date: 2017-06-28T08:37:31Z
[SPARK-18006][SQL] Make sure date or timestamp related predicate is pushed down correctly
----
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by SharpRay <gi...@git.apache.org>.
Github user SharpRay commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125163581
--- Diff: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala ---
@@ -198,4 +198,49 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSQLCo
val types = rows(0).toSeq.map(x => x.getClass.toString)
assert(types(1).equals("class java.sql.Timestamp"))
}
+
+ test("SPARK-18004: Make sure date or timestamp related predicate is pushed down correctly") {
+ val props = new Properties()
+ props.put("oracle.jdbc.mapDateToTimestamp", "false")
+
+ val schema = StructType(Seq(
+ StructField("date_type", DateType, true),
+ StructField("timestamp_type", TimestampType, true)
+ ))
+
+ val tableName = "test_date_timestamp_pushdown"
+ val dateVal = Date.valueOf("2017-06-22")
+ val timestampVal = Timestamp.valueOf("2017-06-22 21:30:07")
+
+ val data = spark.sparkContext.parallelize(Seq(
+ Row(dateVal, timestampVal)
+ ))
+
+ val dfWrite = spark.createDataFrame(data, schema)
+ dfWrite.write.jdbc(jdbcUrl, tableName, props)
+
+ val dfRead = spark.read.jdbc(jdbcUrl, tableName, props)
+
+ val millis = System.currentTimeMillis()
+ val dt = new java.sql.Date(millis)
+ val ts = new java.sql.Timestamp(millis)
+
+ // Query Oracle table with date and timestamp predicates
+ // which should be pushed down to Oracle.
+ val df = dfRead.filter(dfRead.col("date_type").lt(dt))
+ .filter(dfRead.col("timestamp_type").lt(ts))
+
+ val metadata = df.queryExecution.sparkPlan.metadata
+ // The "PushedFilters" part should be exist in Datafrome's
+ // physical plan and the existence of right literals in
+ // "PushedFilters" is used to prove that the predicates
+ // pushing down have been effective.
+ assert(metadata.get("PushedFilters").ne(None))
+ assert(metadata("PushedFilters").contains(dt.toString))
+ assert(metadata("PushedFilters").contains(ts.toString))
+
+ val row = df.collect()(0)
+ assert(row.getDate(0).equals(dateVal))
+ assert(row.getTimestamp(1).equals(timestampVal))
+ }
--- End diff --
So sorry to reply so late. I added the docker test to confirm the date/timestamp-related predicates can be pushed down to Oracle and executed correctly. Before providing this patch , this kind of predicates can be pushed down to Oracle but can not be executed.
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79043 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79043/testReport)** for PR 18451 at commit [`8963b14`](https://github.com/apache/spark/commit/8963b140d14ba45bfbcc41617ebed34777678173).
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18451
Thanks! Merging to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #78991 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78991/testReport)** for PR 18451 at commit [`770bf9c`](https://github.com/apache/spark/commit/770bf9c80a3f0411d97fa397b2e162af76d9a022).
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79029 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79029/testReport)** for PR 18451 at commit [`0d547d2`](https://github.com/apache/spark/commit/0d547d2b7346b53066b97ab0a950f11813f41f45).
* 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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125174365
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala ---
@@ -68,5 +68,15 @@ private case object OracleDialect extends JdbcDialect {
case _ => None
}
+ override def compileValue(value: Any): Any = value match {
+ // Special treatment for the literal representations of
+ // Oracle's Timestamp/Date types.
+ case stringValue: String => s"'${escapeSql(stringValue)}'"
+ case timestampValue: Timestamp => "{ts '" + timestampValue + "'}"
--- End diff --
Add one line before this.
> The JDBC drivers support date literals in SQL statements written in the format: {d 'yyyy-mm-dd'} and timestamp literals in SQL statements written in the format: {ts 'yyyy-mm-dd hh:mm:ss.f...'}. For details, see 'Oracle Database JDBC Developer’s Guide and Reference, 11g Release 1 (11.1)' Appendix A Reference Information
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79029 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79029/testReport)** for PR 18451 at commit [`0d547d2`](https://github.com/apache/spark/commit/0d547d2b7346b53066b97ab0a950f11813f41f45).
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by SharpRay <gi...@git.apache.org>.
Github user SharpRay commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125177864
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -124,6 +126,29 @@ abstract class JdbcDialect extends Serializable {
}
/**
+ * Make string literal in SQL valid.
+ * @param value The string to be escaped.
+ * @return Escaped string.
+ */
+ @Since("2.3.0")
+ protected[jdbc] def escapeSql(value: String): String =
+ if (value == null) null else StringUtils.replace(value, "'", "''")
--- End diff --
I am very glad to be involved!
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79043/
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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79030 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79030/testReport)** for PR 18451 at commit [`9808e63`](https://github.com/apache/spark/commit/9808e63a7a2055173e3e75a555e3a0d846e08dea).
* 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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125174262
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -124,6 +126,29 @@ abstract class JdbcDialect extends Serializable {
}
/**
+ * Make string literal in SQL valid.
+ * @param value The string to be escaped.
+ * @return Escaped string.
+ */
+ @Since("2.3.0")
+ protected[jdbc] def escapeSql(value: String): String =
+ if (value == null) null else StringUtils.replace(value, "'", "''")
--- End diff --
So far, it only covers single quotes. Actually, this is not complete. We need to improve it in the future PRs. If you have time, feel free to submit PRs.
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79043 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79043/testReport)** for PR 18451 at commit [`8963b14`](https://github.com/apache/spark/commit/8963b140d14ba45bfbcc41617ebed34777678173).
* 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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125174270
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -124,6 +126,29 @@ abstract class JdbcDialect extends Serializable {
}
/**
+ * Make string literal in SQL valid.
+ * @param value The string to be escaped.
+ * @return Escaped string.
+ */
+ @Since("2.3.0")
+ protected[jdbc] def escapeSql(value: String): String =
+ if (value == null) null else StringUtils.replace(value, "'", "''")
--- End diff --
```
def escapeSql(value: String): String = {
if (value == null) null else StringUtils.replace(value, "'", "''")}
}
```
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SharpRay <gi...@git.apache.org>.
Github user SharpRay commented on the issue:
https://github.com/apache/spark/pull/18451
Thank you so much @gatorsmile . I learned a lot by communicating with you. I will continue to try to contribute more code. Thank you!
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #79030 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/79030/testReport)** for PR 18451 at commit [`9808e63`](https://github.com/apache/spark/commit/9808e63a7a2055173e3e75a555e3a0d846e08dea).
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SharpRay <gi...@git.apache.org>.
Github user SharpRay commented on the issue:
https://github.com/apache/spark/pull/18451
@gatorsmile I added some descriptions which included:
1. The description of the `compileValue` method in JdbcDialect.
2. The description of the `escapeSql` method in OracleDialect.
3. The description of why the `compileValue` method is overridden in OracleDialect.
Thank you!
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79029/
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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18451
ok to test
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125078775
--- Diff: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala ---
@@ -198,4 +198,49 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSQLCo
val types = rows(0).toSeq.map(x => x.getClass.toString)
assert(types(1).equals("class java.sql.Timestamp"))
}
+
+ test("SPARK-18004: Make sure date or timestamp related predicate is pushed down correctly") {
+ val props = new Properties()
+ props.put("oracle.jdbc.mapDateToTimestamp", "false")
+
+ val schema = StructType(Seq(
+ StructField("date_type", DateType, true),
+ StructField("timestamp_type", TimestampType, true)
+ ))
+
+ val tableName = "test_date_timestamp_pushdown"
+ val dateVal = Date.valueOf("2017-06-22")
+ val timestampVal = Timestamp.valueOf("2017-06-22 21:30:07")
+
+ val data = spark.sparkContext.parallelize(Seq(
+ Row(dateVal, timestampVal)
+ ))
+
+ val dfWrite = spark.createDataFrame(data, schema)
+ dfWrite.write.jdbc(jdbcUrl, tableName, props)
+
+ val dfRead = spark.read.jdbc(jdbcUrl, tableName, props)
+
+ val millis = System.currentTimeMillis()
+ val dt = new java.sql.Date(millis)
+ val ts = new java.sql.Timestamp(millis)
+
+ // Query Oracle table with date and timestamp predicates
+ // which should be pushed down to Oracle.
+ val df = dfRead.filter(dfRead.col("date_type").lt(dt))
+ .filter(dfRead.col("timestamp_type").lt(ts))
+
+ val metadata = df.queryExecution.sparkPlan.metadata
+ // The "PushedFilters" part should be exist in Datafrome's
+ // physical plan and the existence of right literals in
+ // "PushedFilters" is used to prove that the predicates
+ // pushing down have been effective.
+ assert(metadata.get("PushedFilters").ne(None))
+ assert(metadata("PushedFilters").contains(dt.toString))
+ assert(metadata("PushedFilters").contains(ts.toString))
+
+ val row = df.collect()(0)
+ assert(row.getDate(0).equals(dateVal))
+ assert(row.getTimestamp(1).equals(timestampVal))
+ }
--- End diff --
Just want to confirm whether you run the docker test? 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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125174367
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala ---
@@ -68,5 +68,15 @@ private case object OracleDialect extends JdbcDialect {
case _ => None
}
+ override def compileValue(value: Any): Any = value match {
+ // Special treatment for the literal representations of
+ // Oracle's Timestamp/Date types.
--- End diff --
Remove this line
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125174249
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -124,6 +126,29 @@ abstract class JdbcDialect extends Serializable {
}
/**
+ * Make string literal in SQL valid.
--- End diff --
`Make string literal in SQL valid.`
->
`Escape special characters in SQL string literals.`
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/18451
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/79030/
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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125079452
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -123,6 +125,20 @@ abstract class JdbcDialect extends Serializable {
def beforeFetch(connection: Connection, properties: Map[String, String]): Unit = {
}
+ protected def escapeSql(value: String): String =
+ if (value == null) null else StringUtils.replace(value, "'", "''")
+
+ /**
+ * Converts value to SQL expression.
+ */
+ def compileValue(value: Any): Any = value match {
--- End diff --
Could you follow the other functions in this file? Add `@param` and `@Since`
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by SharpRay <gi...@git.apache.org>.
Github user SharpRay commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125177877
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -124,6 +126,29 @@ abstract class JdbcDialect extends Serializable {
}
/**
+ * Make string literal in SQL valid.
--- End diff --
Ok. I will do 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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78991/
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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/18451
**[Test build #78991 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/78991/testReport)** for PR 18451 at commit [`770bf9c`](https://github.com/apache/spark/commit/770bf9c80a3f0411d97fa397b2e162af76d9a022).
* 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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125079729
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala ---
@@ -68,5 +68,13 @@ private case object OracleDialect extends JdbcDialect {
case _ => None
}
+ override def compileValue(value: Any): Any = value match {
+ case stringValue: String => s"'${escapeSql(stringValue)}'"
+ case timestampValue: Timestamp => "{ts '" + timestampValue + "'}"
--- End diff --
Please add description to explain why we did 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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
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 issue #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/18451
Generally, it looks good to me. Thank you!
---
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 #18451: [SPARK-18004][SQL] Make sure the date or timestamp relat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18451
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 #18451: [SPARK-18004][SQL] Make sure the date or timestam...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18451#discussion_r125079882
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -123,6 +125,20 @@ abstract class JdbcDialect extends Serializable {
def beforeFetch(connection: Connection, properties: Map[String, String]): Unit = {
}
+ protected def escapeSql(value: String): String =
+ if (value == null) null else StringUtils.replace(value, "'", "''")
--- End diff --
Style issue.
```
protected def escapeSql(value: String): String = {
if (value == null) null else StringUtils.replace(value, "'", "''")}
}
```
---
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