You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yaooqinn (via GitHub)" <gi...@apache.org> on 2026/01/28 12:20:21 UTC
[PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
yaooqinn opened a new pull request, #54034:
URL: https://github.com/apache/spark/pull/54034
### What changes were proposed in this pull request?
This PR adds support for the IGNORE NULLS and RESPECT NULLS clauses for `array_agg` and `collect_list` aggregate functions.
### Why are the changes needed?
The SQL standard and many databases (PostgreSQL, Snowflake, DuckDB, etc.) support the IGNORE NULLS / RESPECT NULLS syntax for aggregate functions. Currently, Spark only supports this syntax for window functions like `first`, `last`, `lead`, `lag`, and `nth_value`.
By adding this support to `array_agg` and `collect_list`, users can explicitly control whether null values should be included in the resulting array:
- `array_agg(col) IGNORE NULLS` - skips null values (default behavior)
- `array_agg(col) RESPECT NULLS` - includes null values in the result
### Implementation Details
1. Added `ignoreNulls: Boolean = true` parameter to `CollectList` class
2. `array_agg` now uses `CollectList` as they have identical behavior
3. Changed `UnresolvedFunction.ignoreNulls` from `Boolean` to `Option[Boolean]` to distinguish between:
- `None`: no clause specified (use function's default)
- `Some(true)`: IGNORE NULLS explicitly specified
- `Some(false)`: RESPECT NULLS explicitly specified
4. Consolidated ignoreNulls resolution logic in `FunctionResolution` with shared `resolveIgnoreNulls` and `applyIgnoreNulls` methods
### Does this PR introduce _any_ user-facing change?
Yes. Users can now use IGNORE NULLS / RESPECT NULLS with array_agg and collect_list:
```sql
SELECT array_agg(col IGNORE NULLS) FROM table;
SELECT collect_list(col RESPECT NULLS) OVER (PARTITION BY id) FROM table;
```
### How was this patch tested?
Added unit tests in DataFrameAggregateSuite:
- `array_agg and collect_list skip nulls by default`
- `array_agg with IGNORE NULLS explicitly skips nulls`
- `array_agg with RESPECT NULLS preserves nulls`
### Was this patch authored or co-authored using generative AI tooling?
Yes, GitHub Copilot was used to assist with this implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #54034:
URL: https://github.com/apache/spark/pull/54034#discussion_r2742203432
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -258,14 +228,46 @@ class FunctionResolution(
"FILTER clause"
)
}
- if (u.ignoreNulls) {
+ if (u.ignoreNulls.isDefined) {
throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
func.prettyName,
"IGNORE NULLS"
)
}
}
+ /**
+ * Resolves the IGNORE NULLS / RESPECT NULLS clause for a function.
+ * If ignoreNulls is defined, applies it to the function; otherwise returns unchanged.
+ */
+ private def resolveIgnoreNulls[T <: Expression](func: T, ignoreNulls: Option[Boolean]): T = {
+ ignoreNulls.map(applyIgnoreNulls(func, _)).getOrElse(func)
+ }
+
+ /**
+ * Applies the IGNORE NULLS / RESPECT NULLS clause to functions that support it.
+ * Returns the modified function if supported, throws error otherwise.
+ */
+ private def applyIgnoreNulls[T <: Expression](func: T, ignoreNulls: Boolean): T = {
+ val result = func match {
+ // Window functions
+ case nthValue: NthValue => nthValue.copy(ignoreNulls = ignoreNulls)
+ case lead: Lead => lead.copy(ignoreNulls = ignoreNulls)
+ case lag: Lag => lag.copy(ignoreNulls = ignoreNulls)
+ // Aggregate functions
+ case first: First => first.copy(ignoreNulls = ignoreNulls)
+ case last: Last => last.copy(ignoreNulls = ignoreNulls)
+ case anyValue: AnyValue => anyValue.copy(ignoreNulls = ignoreNulls)
+ case collectList: CollectList => collectList.copy(ignoreNulls = ignoreNulls)
+ case _ =>
+ throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
Review Comment:
previously we only fail if `ignoreNulls = true`, shall we keep this behavior?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3824114999
Merged to master, thank you @cloud-fan @dongjoon-hyun @peter-toth. the test failure is known issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn closed pull request #54034: [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list
URL: https://github.com/apache/spark/pull/54034
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3811045174
- SPARK-27974 introduced array_agg but the ANSI 'including nulls' rule is not applied
- ignore nulls is default for these functions, but a bit inconsistent with all other functions w/ ignore null support.
Addressing these could be difficult w/o breaking change, so the behavior stays as-is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #54034:
URL: https://github.com/apache/spark/pull/54034#discussion_r2744728995
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala:
##########
@@ -107,15 +115,39 @@ abstract class Collect[T <: Growable[Any] with Iterable[Any]] extends TypedImper
case class CollectList(
child: Expression,
mutableAggBufferOffset: Int = 0,
- inputAggBufferOffset: Int = 0) extends Collect[mutable.ArrayBuffer[Any]]
+ inputAggBufferOffset: Int = 0,
+ ignoreNulls: Boolean = true) extends Collect[mutable.ArrayBuffer[Any]]
with UnaryLike[Expression] {
- def this(child: Expression) = this(child, 0, 0)
+ def this(child: Expression) = this(child, 0, 0, true)
+
+ // Buffer can contain nulls when ignoreNulls is false (RESPECT NULLS)
+ override protected def bufferContainsNull: Boolean = !ignoreNulls
override lazy val bufferElementType = child.dataType
override def convertToBufferElement(value: Any): Any = InternalRow.copyValue(value)
+ override def update(
+ buffer: mutable.ArrayBuffer[Any],
+ input: InternalRow): mutable.ArrayBuffer[Any] = {
+ val value = child.eval(input)
+ if (ignoreNulls) {
+ // Hive behavior: skip null values
+ if (value != null) {
+ buffer += convertToBufferElement(value)
+ }
+ } else {
+ // ANSI behavior: preserve null values
+ if (value != null) {
+ buffer += convertToBufferElement(value)
+ } else {
+ buffer += null
+ }
+ }
Review Comment:
This looks a little overcomplicated. Shall we simplify this logic like the following, @yaooqinn ?
**BEFORE**
```java
if (ignoreNulls) {
// Hive behavior: skip null values
if (value != null) {
buffer += convertToBufferElement(value)
}
} else {
// ANSI behavior: preserve null values
if (value != null) {
buffer += convertToBufferElement(value)
} else {
buffer += null
}
}
```
**AFTER**
```java
if (value != null) {
buffer += convertToBufferElement(value);
} else if (!ignoreNulls) { // ANSI behavior: preserve null values
buffer += null;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3822012383
cc @peter-toth , too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3822071752
> The ignoreNull=true has been the AS-IS Apache Spark behavior for a long time, isn't it?
Yes, it is. I have revised those Hive-v.s.-ANSI wordings too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3811001069
## JIRA Issue Information
=== Improvement SPARK-55256 ===
Summary: [SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list
Assignee: None
Status: Open
Affected: ["4.2.0"]
---
*This comment was automatically generated by GitHub Actions*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #54034:
URL: https://github.com/apache/spark/pull/54034#discussion_r2742211856
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala:
##########
@@ -54,6 +54,10 @@ abstract class Collect[T <: Growable[Any] with Iterable[Any]] extends TypedImper
protected def convertToBufferElement(value: Any): Any
+ // Subclasses can override this to allow nulls in buffer
+ // (e.g., CollectList with ignoreNulls=false)
+ protected def bufferContainsNull: Boolean = false
Review Comment:
this base class has `override def dataType: DataType = ArrayType(child.dataType, false)`, shall we update it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #54034:
URL: https://github.com/apache/spark/pull/54034#discussion_r2744450474
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -258,14 +228,46 @@ class FunctionResolution(
"FILTER clause"
)
}
- if (u.ignoreNulls) {
+ if (u.ignoreNulls.isDefined) {
throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
func.prettyName,
"IGNORE NULLS"
)
}
}
+ /**
+ * Resolves the IGNORE NULLS / RESPECT NULLS clause for a function.
+ * If ignoreNulls is defined, applies it to the function; otherwise returns unchanged.
+ */
+ private def resolveIgnoreNulls[T <: Expression](func: T, ignoreNulls: Option[Boolean]): T = {
+ ignoreNulls.map(applyIgnoreNulls(func, _)).getOrElse(func)
+ }
+
+ /**
+ * Applies the IGNORE NULLS / RESPECT NULLS clause to functions that support it.
+ * Returns the modified function if supported, throws error otherwise.
+ */
+ private def applyIgnoreNulls[T <: Expression](func: T, ignoreNulls: Boolean): T = {
+ val result = func match {
+ // Window functions
+ case nthValue: NthValue => nthValue.copy(ignoreNulls = ignoreNulls)
+ case lead: Lead => lead.copy(ignoreNulls = ignoreNulls)
+ case lag: Lag => lag.copy(ignoreNulls = ignoreNulls)
+ // Aggregate functions
+ case first: First => first.copy(ignoreNulls = ignoreNulls)
+ case last: Last => last.copy(ignoreNulls = ignoreNulls)
+ case anyValue: AnyValue => anyValue.copy(ignoreNulls = ignoreNulls)
+ case collectList: CollectList => collectList.copy(ignoreNulls = ignoreNulls)
+ case _ =>
+ throw QueryCompilationErrors.functionWithUnsupportedSyntaxError(
Review Comment:
This makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3822060445
Thanks @dongjoon-hyun! Good suggestion - I've simplified the logic as you proposed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-55256][SQL] Support IGNORE NULLS / RESPECT NULLS for array_agg and collect_list [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #54034:
URL: https://github.com/apache/spark/pull/54034#issuecomment-3822121683
Thank you so much again!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org