sunchao commented on code in PR #58117:
URL: https://github.com/apache/spark/pull/58117#discussion_r4428978935
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitmapExpressions.scala:
##########
@@ -375,6 +383,83 @@ case class BitmapXor(left: Expression, right: Expression)
extends BitmapBinaryEx
copy(left = newLeft, right = newRight)
}
+@ExpressionDescription(
+ usage = """
+ _FUNC_(bitmap, bit_position) - Returns true if the bit at the given
bucket-local position is
+ set in the bitmap, false otherwise.
+ """,
+ arguments = """
+ Arguments:
+ * bitmap - The bitmap to test.
+ An expression that evaluates to a binary. A NULL bitmap produces a
NULL result.
+ * bit_position - The bucket-local bit position to test.
+ An expression that evaluates to a numeric value and is cast to a long.
A NULL position
+ produces a NULL result. A negative position, a position at or above
32768, or a position
+ beyond the actual bitmap length returns false. To query an original
value, use
+ bitmap_bit_position(value) and match bitmap_bucket_number(value) to
the bitmap bucket.
+ """,
+ examples = """
+ Examples:
+ > SELECT _FUNC_(X '01', 0L);
+ true
+ > SELECT _FUNC_(X '01', 1L);
+ false
+ > SELECT _FUNC_(X '10', 4L);
+ true
+ """,
+ since = "4.4.0",
+ group = "misc_funcs")
+case class BitmapContains(left: Expression, right: Expression)
+ extends BinaryExpression with RuntimeReplaceable {
+
+ override def checkInputDataTypes(): TypeCheckResult = {
+ if (left.dataType != BinaryType && left.dataType != NullType) {
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(BinaryType),
+ "inputSql" -> toSQLExpr(left),
+ "inputType" -> toSQLType(left.dataType)
+ )
+ )
+ } else if (!right.dataType.isInstanceOf[NumericType] && right.dataType !=
NullType) {
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(1),
+ "requiredType" -> toSQLType(LongType),
+ "inputSql" -> toSQLExpr(right),
+ "inputType" -> toSQLType(right.dataType)
+ )
+ )
+ } else {
+ TypeCheckSuccess
+ }
+ }
+
+ override def dataType: DataType = BooleanType
+
+ override def prettyName: String = "bitmap_contains"
+
+ override lazy val replacement: Expression = {
+ // StaticInvoke needs an exact BinaryType child, including for a bare NULL
literal.
+ val bitmap = if (left.dataType == NullType) Cast(left, BinaryType) else
left
+ StaticInvoke(
+ classOf[BitmapExpressionUtils],
+ BooleanType,
+ "bitmapContains",
+ Seq(bitmap, Cast(right, LongType)),
Review Comment:
**A shared position can throw before either NULL bitmap is checked.** With
ANSI mode and default subexpression elimination, this query raises
`CAST_OVERFLOW` under `CODEGEN_ONLY`.
```sql
WITH inputs AS (
SELECT IF(id = 0, NULL, X'01') AS b1, IF(id = 0, NULL, X'02') AS b2,
IF(id = 0, 18446744073709551617BD, 0BD) AS p
FROM range(2)
)
SELECT bitmap_contains(b1, p), bitmap_contains(b2, p) FROM inputs
```
The same query returns `(NULL, NULL)` and `(true, false)` with `NO_CODEGEN`,
or with codegen on and only subexpression elimination disabled. The generated
projection hoists the shared `CAST(p AS BIGINT)` ahead of both invocations. The
single-call regression passes because there’s no shared cast to hoist.
Constant folding can bypass the same guard even with one call. `SELECT
bitmap_contains(IF(id = 0, NULL, X'01'), 18446744073709551617BD) FROM range(1)`
throws during optimization and returns `NULL` if only `ConstantFolding` is
excluded. Could the replacement express that the position is evaluated
conditionally, so both optimizations respect the bitmap’s NULL check? Both
queries would make good regressions alongside the existing non-NULL overflow
control.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -182,7 +182,8 @@ trait InvokeLike extends Expression with NonSQLExpression
with ImplicitCastInput
var i = 0
val len = arguments.length
var resultNull = false
- while (i < len) {
+ // Short-circuit after a result-determining null to match prepareArguments.
+ while (i < len && !resultNull) {
Review Comment:
**Short-circuiting leaves the previous row’s objects in `evaluatedArgs`.**
When the first argument is NULL, this loop exits without overwriting the
remaining slots in the reusable array. This also affects existing functions. I
exercised the interpreted replacement for binary `contains` with 12 expression
instances, each receiving an 8 MiB second argument once and then all-NULL
inputs. After the last all-NULL row, the arrays held 96 MiB through those
skipped slots on this head and 0 bytes on the PR base. A single expression
similarly retained its payload through 1,000 all-NULL rows.
Could we clear the skipped reference slots on the NULL return path, without
evaluating the skipped expressions? The `NewInstance.eval` loop below has the
same lifetime issue.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitmapExpressions.scala:
##########
@@ -375,6 +383,83 @@ case class BitmapXor(left: Expression, right: Expression)
extends BitmapBinaryEx
copy(left = newLeft, right = newRight)
}
+@ExpressionDescription(
+ usage = """
+ _FUNC_(bitmap, bit_position) - Returns true if the bit at the given
bucket-local position is
+ set in the bitmap, false otherwise.
+ """,
+ arguments = """
+ Arguments:
+ * bitmap - The bitmap to test.
+ An expression that evaluates to a binary. A NULL bitmap produces a
NULL result.
+ * bit_position - The bucket-local bit position to test.
+ An expression that evaluates to a numeric value and is cast to a long.
A NULL position
+ produces a NULL result. A negative position, a position at or above
32768, or a position
+ beyond the actual bitmap length returns false. To query an original
value, use
+ bitmap_bit_position(value) and match bitmap_bucket_number(value) to
the bitmap bucket.
+ """,
+ examples = """
+ Examples:
+ > SELECT _FUNC_(X '01', 0L);
+ true
+ > SELECT _FUNC_(X '01', 1L);
+ false
+ > SELECT _FUNC_(X '10', 4L);
+ true
+ """,
+ since = "4.4.0",
+ group = "misc_funcs")
+case class BitmapContains(left: Expression, right: Expression)
+ extends BinaryExpression with RuntimeReplaceable {
+
+ override def checkInputDataTypes(): TypeCheckResult = {
+ if (left.dataType != BinaryType && left.dataType != NullType) {
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(0),
+ "requiredType" -> toSQLType(BinaryType),
+ "inputSql" -> toSQLExpr(left),
+ "inputType" -> toSQLType(left.dataType)
+ )
+ )
+ } else if (!right.dataType.isInstanceOf[NumericType] && right.dataType !=
NullType) {
+ DataTypeMismatch(
+ errorSubClass = "UNEXPECTED_INPUT_TYPE",
+ messageParameters = Map(
+ "paramIndex" -> ordinalNumber(1),
+ "requiredType" -> toSQLType(LongType),
Review Comment:
**Report the position’s accepted type in the diagnostic.**
`bitmap_contains(X'01', '0')` says the second parameter requires `BIGINT`, but
`bitmap_contains(X'01', CAST(0.1 AS DECIMAL(2,1)))` is valid and returns
`true`. This check accepts every `NumericType`. Could it use
`toSQLType(NumericType)` and expect `NUMERIC` in the negative test, as
`uniform` does for its numeric parameter?
--
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]
