[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/17896 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17896#discussion_r115418094
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2457,6 +2457,19 @@ object CleanupAliases extends Rule[LogicalPlan] {
}
/**
+ * Ignore event time watermark in batch query, which is only supported in
Structured Streaming.
+ * TODO: add this rule into analyzer rule list.
+ */
+object CheckEventTimeWatermark extends Rule[LogicalPlan] {
--- End diff --
I see. The current approach is good to me then. Could you rename it, 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user uncleGen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17896#discussion_r115415803
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2457,6 +2457,19 @@ object CleanupAliases extends Rule[LogicalPlan] {
}
/**
+ * Ignore event time watermark in batch query, which is only supported in
Structured Streaming.
+ * TODO: add this rule into analyzer rule list.
+ */
+object CheckEventTimeWatermark extends Rule[LogicalPlan] {
+ override def apply(plan: LogicalPlan): LogicalPlan = plan transform {
+case EventTimeWatermark(_, _, child) if !child.isStreaming =>
+ logWarning("EventTime watermark is only supported in Structured
Streaming but found " +
--- End diff --
got
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user uncleGen commented on a diff in the pull request:
https://github.com/apache/spark/pull/17896#discussion_r115415668
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2457,6 +2457,19 @@ object CleanupAliases extends Rule[LogicalPlan] {
}
/**
+ * Ignore event time watermark in batch query, which is only supported in
Structured Streaming.
+ * TODO: add this rule into analyzer rule list.
+ */
+object CheckEventTimeWatermark extends Rule[LogicalPlan] {
--- End diff --
@zsxwing This pr does some prepare work before we add
`EliminateEventTimeWatermark ` into `Analyzer.batches`. Could you please take a
review?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17896#discussion_r115349969
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2457,6 +2457,19 @@ object CleanupAliases extends Rule[LogicalPlan] {
}
/**
+ * Ignore event time watermark in batch query, which is only supported in
Structured Streaming.
+ * TODO: add this rule into analyzer rule list.
+ */
+object CheckEventTimeWatermark extends Rule[LogicalPlan] {
+ override def apply(plan: LogicalPlan): LogicalPlan = plan transform {
+case EventTimeWatermark(_, _, child) if !child.isStreaming =>
+ logWarning("EventTime watermark is only supported in Structured
Streaming but found " +
--- End diff --
this warning log is not needed. Could you just update `withWatermark` doc
and says
```
Using `withWatermark` on a non-streaming [[Dataset]] is no-op`.
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/17896#discussion_r115348902
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -2457,6 +2457,19 @@ object CleanupAliases extends Rule[LogicalPlan] {
}
/**
+ * Ignore event time watermark in batch query, which is only supported in
Structured Streaming.
+ * TODO: add this rule into analyzer rule list.
+ */
+object CheckEventTimeWatermark extends Rule[LogicalPlan] {
--- End diff --
Could you rename it to `EliminateEventTimeWatermark` and add it into
`Analyzer.batches` as one of `Substitution` rules?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #17896: [SPARK-20373][SQL][SS] Batch queries with 'Datase...
GitHub user uncleGen opened a pull request: https://github.com/apache/spark/pull/17896 [SPARK-20373][SQL][SS] Batch queries with 'Dataset/DataFrame.withWatermark()` does not execute ## What changes were proposed in this pull request? Any Dataset/DataFrame batch query with the operation `withWatermark` does not execute because the batch planner does not have any rule to explicitly handle the EventTimeWatermark logical plan. The right solution is to simply remove the plan node, as the watermark should not affect any batch query in any way. Changes: - In this PR, we add a new rule `CheckEventTimeWatermark` to check if we need to ignore the event time watermark. We will ignore watermark in any batch query. Followups: - Add `CheckEventTimeWatermark` into analyzer rule list. We can not add this rule into analyzer directly, because streaming query will be copied to a internal batch query in every trigger, and the rule will be applied to this internal batch query mistakenly. IIUC, we should refactor related codes to better define a query is batch or streaming. Right? Others: - A typo fix in example. ## How was this patch tested? add new unit test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/uncleGen/spark SPARK-20373 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/17896.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 #17896 commit 563721241851751c2bb1736161febe73b8abba3b Author: uncleGen Date: 2017-05-08T03:19:35Z Ignore event time watermark in batch query. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
