Skip to content

Commit a50fbf7

Browse files
nikola-jovicevic-dbcloud-fan
authored andcommitted
[SPARK-52869][SQL] Add FrameLessOffsetWindowFunction validation to validateResolvedWindowExpression for reuse in single-pass analyzer
### What changes were proposed in this pull request? A check if `FrameLessOffsetWindowFunction` is compatible with its frame (it has to be ordered and can't be a `SpecifiedWindowFrame` other than an ordered frame) is added to `WindowResolver.validateResolvedWindowExpression`. ### Why are the changes needed? The same repeating logic is required both in the fixed-point and single-pass analyzer implementations. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Using existing unit tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #51559 from nikola-jovicevic-db/SPARK-52869. Authored-by: Nikola Jovićević <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent 1a8c26c commit a50fbf7

File tree

2 files changed

+26
-10
lines changed

2 files changed

+26
-10
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -437,15 +437,6 @@ trait CheckAnalysis extends LookupCatalog with QueryErrorsBase with PlanToString
437437
errorClass = "WINDOW_FUNCTION_WITHOUT_OVER_CLAUSE",
438438
messageParameters = Map("funcName" -> toSQLExpr(w)))
439439

440-
case w @ WindowExpression(wf: FrameLessOffsetWindowFunction,
441-
WindowSpecDefinition(_, order, frame: SpecifiedWindowFrame))
442-
if order.isEmpty || !frame.isOffset =>
443-
w.failAnalysis(
444-
errorClass = "WINDOW_FUNCTION_AND_FRAME_MISMATCH",
445-
messageParameters = Map(
446-
"funcName" -> toSQLExpr(wf),
447-
"windowExpr" -> toSQLExpr(w)))
448-
449440
case agg @ AggregateExpression(listAgg: ListAgg, _, _, _, _)
450441
if agg.isDistinct && listAgg.needSaveOrderValue =>
451442
throw QueryCompilationErrors.functionAndOrderExpressionMismatchError(

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/WindowResolution.scala

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,40 @@ object WindowResolution {
107107
*
108108
* By checking the type and configuration of [[WindowExpression.windowFunction]] it enforces the
109109
* following rules:
110+
* - Disallows [[FrameLessOffsetWindowFunction]] (e.g. [[Lag]]) without defined ordering or
111+
* one with a frame which is defined as something other than an offset frame (e.g.
112+
* `ROWS BETWEEN` is logically incompatible with offset functions).
110113
* - Disallows distinct aggregate expressions in window functions.
111-
* - Disallows use of certain aggregate functions - [[ListaAgg]], [[PercentileCont]],
114+
* - Disallows use of certain aggregate functions - [[ListAgg]], [[PercentileCont]],
112115
* [[PercentileDisc]], [[Median]]
113116
* - Allows only window functions of following types:
114117
* - [[AggregateExpression]] (non-distinct)
115118
* - [[FrameLessOffsetWindowFunction]]
116119
* - [[AggregateWindowFunction]]
117120
*/
118121
def validateResolvedWindowExpression(windowExpression: WindowExpression): Unit = {
122+
checkWindowFunctionAndFrameMismatch(windowExpression)
123+
checkWindowFunction(windowExpression)
124+
}
125+
126+
def checkWindowFunctionAndFrameMismatch(windowExpression: WindowExpression): Unit = {
127+
windowExpression match {
128+
case _ @ WindowExpression(
129+
windowFunction: FrameLessOffsetWindowFunction,
130+
WindowSpecDefinition(_, order, frame: SpecifiedWindowFrame)
131+
) if order.isEmpty || !frame.isOffset =>
132+
windowExpression.failAnalysis(
133+
errorClass = "WINDOW_FUNCTION_AND_FRAME_MISMATCH",
134+
messageParameters = Map(
135+
"funcName" -> toSQLExpr(windowFunction),
136+
"windowExpr" -> toSQLExpr(windowExpression)
137+
)
138+
)
139+
case _ =>
140+
}
141+
}
142+
143+
def checkWindowFunction(windowExpression: WindowExpression): Unit = {
119144
windowExpression.windowFunction match {
120145
case AggregateExpression(_, _, true, _, _) =>
121146
windowExpression.failAnalysis(

0 commit comments

Comments
 (0)