Skip to content

[SPARK-19903][PYSPARK][SS] window operator miss the watermark metadata of time column - #17371

Closed
uncleGen wants to merge 2 commits into
apache:masterfrom
uncleGen:python-window
Closed

[SPARK-19903][PYSPARK][SS] window operator miss the watermark metadata of time column#17371
uncleGen wants to merge 2 commits into
apache:masterfrom
uncleGen:python-window

Conversation

@uncleGen

@uncleGenuncleGen commented Mar 21, 2017

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

reproduce code:

import sys
from pyspark.sql import SparkSession
from pyspark.sql.functions import explode, split, window
bootstrapServers = sys.argv[1]
subscribeType = sys.argv[2]
topics = sys.argv[3]
spark = SparkSession\
.builder\
.appName("StructuredKafkaWordCount")\
.getOrCreate()
lines = spark\
.readStream\
.format("kafka")\
.option("kafka.bootstrap.servers", bootstrapServers)\
.option(subscribeType, topics)\
.load()\
.selectExpr("CAST(value AS STRING)", "CAST(timestamp AS TIMESTAMP)")
words = lines.select(explode(split(lines.value, ' ')).alias('word'),lines.timestamp)
windowedCounts = words.withWatermark("timestamp", "30 seconds").groupBy(
window(words.timestamp, "30 seconds", "30 seconds"), words.word
).count()
query = windowedCounts\
.writeStream\
.outputMode('append')\
.format('console')\ .option("truncate", "false")\
.start()
query.awaitTermination()

An exception was thrown:

pyspark.sql.utils.AnalysisException: Append output mode not supported when there are streaming aggregations on streaming DataFrames/DataSets without watermark;;
Aggregate [window#32, word#21], [window#32 AS window#26, word#21, count(1) AS count#31L]
+- Filter ((timestamp#16 >= window#32.start) && (timestamp#16 < window#32.end))
+- Expand [ArrayBuffer(named_struct(start, ...]
+- EventTimeWatermark timestamp#16: timestamp, interval 10 seconds
+- Project [word#21, timestamp#16]
+- Generate explode(split(value#15, )), true, false, [word#21]
+- Project [cast(value#1 as string) AS value#15, cast(timestamp#5 as timestamp) AS timestamp#16]
+- StreamingRelation DataSource(org.apache.spark.sql.SparkSession ...]

IIUC, the root cause is: words.withWatermark("timestamp", "30 seconds") add the watermark metadata into time column, but in groupBy( window(words.timestamp, "30 seconds", "30 seconds"), words.word ), the words.timestamp miss the metadata. At last, it failed to pass the check:

use @viirya 's more clear explanation:

For now, after withWatermark, we only update the metadata for the column of event time. The expression id is the same. So once we use the column before adding watermark words.timestamp as grouping expression, it binds to the old attribute before watermarking.

if (watermarkAttributes.isEmpty) {
throwError(
s"$outputMode output mode not supported when there are streaming aggregations on " +
s"streaming DataFrames/DataSets without watermark")(plan)
}

In this pr, pass a UnresolvedAttribute to window instead of a Column

How was this patch tested?

Jenkins


sc = SparkContext._active_spark_context
time_col = _to_java_column(timeColumn)
if isinstance(timeColumn, Column):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, doesn't this break the current API?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, it is OK for current codebase. Am I missing something?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change, you can't pass in a Column. But it is supported for now.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@viirya Sounds reasonable, I pushed an update, take a review please.

@SparkQA

Copy link
Copy Markdown

Test build #74959 has finished for PR 17371 at commit 654c512.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA

Copy link
Copy Markdown

Test build #74967 has finished for PR 17371 at commit 890c6e6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya

Copy link
Copy Markdown
Member

For now, after withWatermark, we only update the metadata for the column of event time. The expression id is the same. So once we use the column before adding watermark words.timestamp as grouping expression, it binds to the old attribute before watermarking.

I am thinking, should we create new expression id for the watermarking column with withWatermark? So we must write the query like:

wordsWithWatermark = words.withWatermark("timestamp", "30 seconds")
windowedCounts = wordsWithWatermark.groupBy(window(wordsWithWatermark.timestamp, "30 seconds", "30 seconds"), wordsWithWatermark.word).count()

@uncleGen

Copy link
Copy Markdown
ContributorAuthor

@viirya Great, you give a more clear explanation.

I am thinking, should we create new expression id for the watermarking column with withWatermark? So we must write the query like:

It really can fix this problem, but not very user-friendly.

@viirya

Copy link
Copy Markdown
Member

IMHO, the output after withWatermark should be new attribute and have new expression id. Maybe @zsxwing@marmbrus have more insights on this?

Btw, does this issue also happen in Scala code?

@marmbrus

Copy link
Copy Markdown
Contributor

I really think the core problem here is that we allow you to use resolved attributes at all in the user API. Unfortunately we are somewhat stuck with that bad decision. Personally, I never use df['col'] and only ever use col("col") since that avoids the problem.

However, I don't think that piecemeal switching to unresolved attributes is a good idea.

@viirya

Copy link
Copy Markdown
Member

Unfortunately, yes, allowing resolved attributes in user API will have this kind of trouble.

However, I don't think that piecemeal switching to unresolved attributes is a good idea.

Agreed. Should we create new attributes after withWatermark to avoid the problem? It might be cumbersome from the user side, however.

@marmbrus

Copy link
Copy Markdown
Contributor

I don't think that will solve the problem though. You will just get a different error message.

@viirya

Copy link
Copy Markdown
Member

yeah, I just tried it. IncrementalExecution will re-new the attribute for each batch. Although we can replace the attribute...

@marmbrus

Copy link
Copy Markdown
Contributor

Can we add an analysis rule that just pulls up missing metadata from attributes in the child? It could run once after other rules.

@HyukjinKwon

Copy link
Copy Markdown
Member

gentle ping @uncleGen, is this PR still active?

@HyukjinKwonHyukjinKwon mentioned this pull request Jul 31, 2017
@asfgitasfgit closed this in 3a45c7fAug 5, 2017
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
## What changes were proposed in this pull request?
This PR proposes to close stale PRs, mostly the same instances with apache#18017Closesapache#14085 - [SPARK-16408][SQL] SparkSQL Added file get Exception: is a directory …
Closesapache#14239 - [SPARK-16593] [CORE] [WIP] Provide a pre-fetch mechanism to accelerate shuffle stage.
Closesapache#14567 - [SPARK-16992][PYSPARK] Python Pep8 formatting and import reorganisation
Closesapache#14579 - [SPARK-16921][PYSPARK] RDD/DataFrame persist()/cache() should return Python context managers
Closesapache#14601 - [SPARK-13979][Core] Killed executor is re spawned without AWS key…
Closesapache#14830 - [SPARK-16992][PYSPARK][DOCS] import sort and autopep8 on Pyspark examples
Closesapache#14963 - [SPARK-16992][PYSPARK] Virtualenv for Pylint and pep8 in lint-python
Closesapache#15227 - [SPARK-17655][SQL]Remove unused variables declarations and definations in a WholeStageCodeGened stage
Closesapache#15240 - [SPARK-17556] [CORE] [SQL] Executor side broadcast for broadcast joins
Closesapache#15405 - [SPARK-15917][CORE] Added support for number of executors in Standalone [WIP]
Closesapache#16099 - [SPARK-18665][SQL] set statement state to "ERROR" after user cancel job
Closesapache#16445 - [SPARK-19043][SQL]Make SparkSQLSessionManager more configurable
Closesapache#16618 - [SPARK-14409][ML][WIP] Add RankingEvaluator
Closesapache#16766 - [SPARK-19426][SQL] Custom coalesce for Dataset
Closesapache#16832 - [SPARK-19490][SQL] ignore case sensitivity when filtering hive partition columns
Closesapache#17052 - [SPARK-19690][SS] Join a streaming DataFrame with a batch DataFrame which has an aggregation may not work
Closesapache#17267 - [SPARK-19926][PYSPARK] Make pyspark exception more user-friendly
Closesapache#17371 - [SPARK-19903][PYSPARK][SS] window operator miss the `watermark` metadata of time column
Closesapache#17401 - [SPARK-18364][YARN] Expose metrics for YarnShuffleService
Closesapache#17519 - [SPARK-15352][Doc] follow-up: add configuration docs for topology-aware block replication
Closesapache#17530 - [SPARK-5158] Access kerberized HDFS from Spark standalone
Closesapache#17854 - [SPARK-20564][Deploy] Reduce massive executor failures when executor count is large (>2000)
Closesapache#17979 - [SPARK-19320][MESOS][WIP]allow specifying a hard limit on number of gpus required in each spark executor when running on mesos
Closesapache#18127 - [SPARK-6628][SQL][Branch-2.1] Fix ClassCastException when executing sql statement 'insert into' on hbase table
Closesapache#18236 - [SPARK-21015] Check field name is not null and empty in GenericRowWit…
Closesapache#18269 - [SPARK-21056][SQL] Use at most one spark job to list files in InMemoryFileIndex
Closesapache#18328 - [SPARK-21121][SQL] Support changing storage level via the spark.sql.inMemoryColumnarStorage.level variable
Closesapache#18354 - [SPARK-18016][SQL][CATALYST][BRANCH-2.1] Code Generation: Constant Pool Limit - Class Splitting
Closesapache#18383 - [SPARK-21167][SS] Set kafka clientId while fetch messages
Closesapache#18414 - [SPARK-21169] [core] Make sure to update application status to RUNNING if executors are accepted and RUNNING after recovery
Closesapache#18432 - resolve com.esotericsoftware.kryo.KryoException
Closesapache#18490 - [SPARK-21269][Core][WIP] Fix FetchFailedException when enable maxReqSizeShuffleToMem and KryoSerializer
Closesapache#18585 - SPARK-21359
Closesapache#18609 - Spark SQL merge small files to big files Update InsertIntoHiveTable.scala
Added:
Closesapache#18308 - [SPARK-21099][Spark Core] INFO Log Message Using Incorrect Executor I…
Closesapache#18599 - [SPARK-21372] spark writes one log file even I set the number of spark_rotate_log to 0
Closesapache#18619 - [SPARK-21397][BUILD]Maven shade plugin adding dependency-reduced-pom.xml to …
Closesapache#18667 - Fix the simpleString used in error messages
Closesapache#18782 - Branch 2.1
Added:
Closesapache#17694 - [SPARK-12717][PYSPARK] Resolving race condition with pyspark broadcasts when using multiple threads
Added:
Closesapache#16456 - [SPARK-18994] clean up the local directories for application in future by annother thread
Closesapache#18683 - [SPARK-21474][CORE] Make number of parallel fetches from a reducer configurable
Closesapache#18690 - [SPARK-21334][CORE] Add metrics reporting service to External Shuffle Server
Added:
Closesapache#18827 - Merge pull request 1 from apache/master
## How was this patch tested?
N/A
Author: hyukjinkwon <gurwls223@gmail.com>
Closesapache#18780 from HyukjinKwon/close-prs.
MaxGekk added a commit to vecbricks/varka that referenced this pull request Sep 3, 2026
…ures for task 37 (#101)
### What changes were proposed in this pull request?
Records a review of [Velox](https://github.com/facebookincubator/velox)'s datetime code (`velox/functions/sparksql/DateTimeFunctions.h`, `velox/functions/lib/{TimeUtils,DateTimeUtil,DateTimeFormatter}.*`, `velox/type/{FastDate.h,TimestampConversion.cpp,Timestamp.cpp}`, `velox/functions/lib/SIMDComparisonUtil.h`, and the sparksql tests) against Varka's calendar family. Documentation only: task 37's row in `PLAN_MILESTONE_4.md` and a `SKILLS.md` section.
**Nothing transfers on performance.** Every Spark-compatible date function converts the day to a `struct tm` through a full civil decomposition and reads one field, one decomposition per row per function. Since May 2026 (Velox PR apache#17371, `FastDate.h`) that decomposition is Neri-Schneider's reference code with era shift 82: the month block task 53 shipped, the `1461 * y / 4 - c + c / 4` and `(979 * m - 2919) / 32` inverse recorded by the `datealgo-rs` review (#97), and the 64-bit year multiply task 49 is waiting for. Their measured gain from the swap was 1.6-1.9x on `month`/`day`, none on `year(date)`. ISO week uses Hinnant's `iso_week.h`; `yearofweek` keeps the boundary corrections task 37 dropped; `next_day` is `start + 1 + floorMod(dow - 1 - start, 7)` as task 33 does. No datetime file contains SIMD; the only SIMD near expressions packs 64 comparison bytes into a bitmask, which Varka gets from `VectorMask.toLong()`.
**What does transfer is test data.** Velox's Spark-compatibility fixtures were written against Spark by people who had to match it exactly:
| set | used for |
|---|---|
| `weekOfYear` (1919-12-31 and 1969-12-31 in week 1, 1960-01-01 in week 53, 0001-01-01 in week 1, 9999-12-31 in week 52, leap years ending on Thu/Fri/Sat) | named in task 37's row as pinned cases to import beside its dense year-boundary sweep |
| `addMonths`, `makeDate` (clamping and rejection cases) | cross-check for tasks 40 and 42, already covered |
| the string-to-date cast's grammar (optional sign, >= 4 year digits, optional `-[m]m`, `-[d]d`, then end / space / `T`) | confirms milestone 4 item 8's fixed-form shape mask sends the right subset to the fallback |
Also noted: Velox's `DateExtractBenchmark`/`FormatDateTimeBenchmark` fuzz dates within 67 years of the epoch, which is both a possible external scalar-engine reference (at the cost of a Velox build) and a fair data-shape argument for milestone 5 item 10's cache question.
### Why are the changes needed?
This is the fourth neighbouring codebase read for the calendar family; each is recorded so it is not read twice. Velox's is the shortest record because its arithmetic is what Varka already has, and the useful residue is a fixture list for a task not yet built.
### Does this PR introduce _any_ user-facing change?
No. Documentation only; no code, no test, no committed benchmark number is modified.
### How was this patch tested?
By reading the sources named above and the merge message of Velox PR apache#17371 for its measurements. No emitter code changes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
MaxGekk added a commit to vecbricks/varka that referenced this pull request Sep 3, 2026
### What changes were proposed in this pull request?
Records a review of [Velox](https://github.com/facebookincubator/velox)'s datetime code (`velox/functions/sparksql/DateTimeFunctions.h`, `velox/functions/lib/{TimeUtils,DateTimeUtil,DateTimeFormatter}.*`, `velox/type/{FastDate.h,TimestampConversion.cpp,Timestamp.cpp}`, `velox/functions/lib/SIMDComparisonUtil.h`, and the sparksql tests) against Varka's calendar family. Documentation only: task 37's row in `PLAN_MILESTONE_4.md` and a `SKILLS.md` section.
**Nothing transfers on performance.** Every Spark-compatible date function converts the day to a `struct tm` through a full civil decomposition and reads one field, one decomposition per row per function. Since May 2026 (Velox PR apache#17371, `FastDate.h`) that decomposition is Neri-Schneider's reference code with era shift 82: the month block task 53 shipped, the `1461 * y / 4 - c + c / 4` and `(979 * m - 2919) / 32` inverse recorded by the `datealgo-rs` review (#97), and the 64-bit year multiply task 49 is waiting for. Their measured gain from the swap was 1.6-1.9x on `month`/`day`, none on `year(date)`. ISO week uses Hinnant's `iso_week.h`; `yearofweek` keeps the boundary corrections task 37 dropped; `next_day` is `start + 1 + floorMod(dow - 1 - start, 7)` as task 33 does. No datetime file contains SIMD; the only SIMD near expressions packs 64 comparison bytes into a bitmask, which Varka gets from `VectorMask.toLong()`.
**What does transfer is test data.** Velox's Spark-compatibility fixtures were written against Spark by people who had to match it exactly:
| set | used for |
|---|---|
| `weekOfYear` (1919-12-31 and 1969-12-31 in week 1, 1960-01-01 in week 53, 0001-01-01 in week 1, 9999-12-31 in week 52, leap years ending on Thu/Fri/Sat) | named in task 37's row as pinned cases to import beside its dense year-boundary sweep |
| `addMonths`, `makeDate` (clamping and rejection cases) | cross-check for tasks 40 and 42, already covered |
| the string-to-date cast's grammar (optional sign, >= 4 year digits, optional `-[m]m`, `-[d]d`, then end / space / `T`) | confirms milestone 4 item 8's fixed-form shape mask sends the right subset to the fallback |
Also noted: Velox's `DateExtractBenchmark`/`FormatDateTimeBenchmark` fuzz dates within 67 years of the epoch, which is both a possible external scalar-engine reference (at the cost of a Velox build) and a fair data-shape argument for milestone 5 item 10's cache question.
### Why are the changes needed?
This is the fourth neighbouring codebase read for the calendar family; each is recorded so it is not read twice. Velox's is the shortest record because its arithmetic is what Varka already has, and the useful residue is a fixture list for a task not yet built.
### Does this PR introduce _any_ user-facing change?
No. Documentation only; no code, no test, no committed benchmark number is modified.
### How was this patch tested?
By reading the sources named above and the merge message of Velox PR apache#17371 for its measurements. No emitter code changes.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@uncleGen@SparkQA@viirya@marmbrus@HyukjinKwon