peter-toth commented on code in PR #57413:
URL: https://github.com/apache/spark/pull/57413#discussion_r3632378585


##########
sql/api/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -12933,7 +12933,8 @@ object functions {
   
//////////////////////////////////////////////////////////////////////////////////////////////
 
   /**
-   * Returns null if the array is null, true if the array contains `value`, 
and false otherwise.
+   * Returns true if the array contains `value`. Returns null if the array is 
null, or if `value`

Review Comment:
   `ArrayContains` extends `BinaryExpression` with `nullIntolerant = true` and 
only defines `nullSafeEval`, so the inherited `eval` returns null whenever 
*either* input is null -- including a null search `value` (`nullSafeEval` never 
runs in that case). So `array_contains(array(1, 2, 3), CAST(NULL AS INT))` 
returns NULL, but this sentence's "otherwise returns false" predicts false -- 
the same "false otherwise" gap this PR is fixing on the array side.
   
   Worth covering the value-null case symmetrically, e.g.:
   
   ```scala
      * Returns true if the array contains `value`. Returns null if the array 
is null, if `value`
      * is null, or if `value` is not found and the array contains a null 
element; otherwise returns
      * false.
   ```
   
   The same applies to the PySpark summary in 
`python/pyspark/sql/functions/builtin.py`, and (if you want all three surfaces 
to match) the SQL `usage` string in `collectionOperations.scala`.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to