Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
zhengruifeng commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2109157257 add it to python API references in https://github.com/apache/spark/pull/46566 -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
gengliangwang commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2108420085 @grundprinzip Shall we create a new jira for this instead of using https://issues.apache.org/jira/browse/SPARK-41794? -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
gengliangwang closed pull request #46434: [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests URL: https://github.com/apache/spark/pull/46434 -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
gengliangwang commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2108398956 Thanks for adding the new function! Merging to master. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on code in PR #46434:
URL: https://github.com/apache/spark/pull/46434#discussion_r1597878145
##
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListenerBus.scala:
##
@@ -121,8 +121,10 @@ class StreamingQueryListenerBus(sparkSession:
SparkSession) extends Logging {
}
} catch {
case e: Exception =>
-logWarning("StreamingQueryListenerBus Handler thread received
exception, all client" +
- " side listeners are removed and handler thread is terminated.", e)
+logWarning(
+ "StreamingQueryListenerBus Handler thread received exception, all
client" +
+" side listeners are removed and handler thread is terminated.",
+ e)
Review Comment:
I did the spark connect auto format and it produced these changes. I'm ok
with reverting them in the worst case but at the same time your suggestions
introduce na manual style adjustment.
LMK
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on code in PR #46434: URL: https://github.com/apache/spark/pull/46434#discussion_r1597877323 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala: ## @@ -150,7 +187,7 @@ case class TryDivide(left: Expression, right: Expression, replacement: Expressio > SELECT _FUNC_(interval 2 year, interval 1 year); 1-0 """, - since = "3.3.0", + since = "4.0.0", Review Comment: Yes. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
HyukjinKwon commented on code in PR #46434:
URL: https://github.com/apache/spark/pull/46434#discussion_r1597739810
##
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##
@@ -451,8 +451,7 @@ object CheckConnectJvmClientCompatibility {
"org.apache.spark.sql.streaming.RemoteStreamingQuery$"),
// Skip client side listener specific class
ProblemFilters.exclude[MissingClassProblem](
-"org.apache.spark.sql.streaming.StreamingQueryListenerBus"
- ),
+"org.apache.spark.sql.streaming.StreamingQueryListenerBus"),
Review Comment:
```suggestion
"org.apache.spark.sql.streaming.StreamingQueryListenerBus"
),
```
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
HyukjinKwon commented on code in PR #46434:
URL: https://github.com/apache/spark/pull/46434#discussion_r1597739768
##
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/streaming/StreamingQueryListenerBus.scala:
##
@@ -121,8 +121,10 @@ class StreamingQueryListenerBus(sparkSession:
SparkSession) extends Logging {
}
} catch {
case e: Exception =>
-logWarning("StreamingQueryListenerBus Handler thread received
exception, all client" +
- " side listeners are removed and handler thread is terminated.", e)
+logWarning(
+ "StreamingQueryListenerBus Handler thread received exception, all
client" +
+" side listeners are removed and handler thread is terminated.",
+ e)
Review Comment:
```suggestion
logWarning("StreamingQueryListenerBus Handler thread received
exception, all client" +
" side listeners are removed and handler thread is terminated.", e)
```
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
HyukjinKwon commented on code in PR #46434: URL: https://github.com/apache/spark/pull/46434#discussion_r1597739657 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala: ## @@ -150,7 +187,7 @@ case class TryDivide(left: Expression, right: Expression, replacement: Expressio > SELECT _FUNC_(interval 2 year, interval 1 year); 1-0 """, - since = "3.3.0", + since = "4.0.0", Review Comment: ```suggestion since = "3.3.0", ``` Is this a mistake? -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2106336680 @gengliangwang @dongjoon-hyun I addressed all comments and added the additional test, PTAL. Thanks! -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on code in PR #46434: URL: https://github.com/apache/spark/pull/46434#discussion_r1597553930 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala: ## @@ -132,6 +132,43 @@ case class TryDivide(left: Expression, right: Expression, replacement: Expressio } } +// scalastyle:off line.size.limit +@ExpressionDescription( + usage = "_FUNC_(dividend, divisor) - Returns the remainder after `expr1`/`expr2`. " + +"`dividend` must be a numeric. `divisor` must be a numeric.", + examples = """ +Examples: + > SELECT _FUNC_(3, 2); + 1 + > SELECT _FUNC_(2L, 2L); + 0 + > SELECT _FUNC_(3.0, 2.0); + 1.0 + > SELECT _FUNC_(1, 0); + NULL + """, + since = "3.2.0", Review Comment: Done -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on code in PR #46434:
URL: https://github.com/apache/spark/pull/46434#discussion_r1597553871
##
sql/core/src/test/scala/org/apache/spark/sql/MathFunctionsSuite.scala:
##
@@ -707,6 +707,11 @@ class MathFunctionsSuite extends QueryTest with
SharedSparkSession {
df1.select(try_divide(make_interval(col("year"), col("month")), lit(0
}
+ test("try_remainder") {
+val df = Seq((10, 3), (5, 5), (5, 0)).toDF("birth", "age")
Review Comment:
Done
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
gengliangwang commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2100956904 @grundprinzip thanks for the work! Let's also mention the new function in https://spark.apache.org/docs/latest/sql-ref-ansi-compliance.html#useful-functions-for-ansi-mode -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
gengliangwang commented on code in PR #46434:
URL: https://github.com/apache/spark/pull/46434#discussion_r1594306390
##
sql/core/src/test/scala/org/apache/spark/sql/MathFunctionsSuite.scala:
##
@@ -707,6 +707,11 @@ class MathFunctionsSuite extends QueryTest with
SharedSparkSession {
df1.select(try_divide(make_interval(col("year"), col("month")), lit(0
}
+ test("try_remainder") {
+val df = Seq((10, 3), (5, 5), (5, 0)).toDF("birth", "age")
Review Comment:
Let's have some end-to-end test with decimal input as well, either in this
test suite or in
`sql/core/src/test/resources/sql-tests/inputs/try_arithmetic.sql`
context: there was a bug found recently
https://github.com/apache/spark/pull/46286
##
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/TryEval.scala:
##
@@ -132,6 +132,43 @@ case class TryDivide(left: Expression, right: Expression,
replacement: Expressio
}
}
+// scalastyle:off line.size.limit
+@ExpressionDescription(
+ usage = "_FUNC_(dividend, divisor) - Returns the remainder after
`expr1`/`expr2`. " +
+"`dividend` must be a numeric. `divisor` must be a numeric.",
+ examples = """
+Examples:
+ > SELECT _FUNC_(3, 2);
+ 1
+ > SELECT _FUNC_(2L, 2L);
+ 0
+ > SELECT _FUNC_(3.0, 2.0);
+ 1.0
+ > SELECT _FUNC_(1, 0);
+ NULL
+ """,
+ since = "3.2.0",
Review Comment:
```suggestion
since = "4.0.0",
```
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
dongjoon-hyun commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2100893973 Thank you. BTW, the recent test failure is relevant? For me, it looks irrelevant. Could you take a look at? ``` [info] *** 1 TEST FAILED *** [error] Failed: Total 10578, Failed 1, Errors 0, Passed 10577, Ignored 29 [error] Failed tests: [error] org.apache.spark.sql.execution.python.PythonStreamingDataSourceSuite [error] (sql / Test / test) sbt.TestsFailedException: Tests unsuccessful [error] Total time: 3020 s (50:20), completed May 8, 2024, 1:48:01 PM ``` -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
grundprinzip commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2099962569 yes, done. -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-41794][SQL] Add `try_remainder` function and re-enable column tests [spark]
dongjoon-hyun commented on PR #46434: URL: https://github.com/apache/spark/pull/46434#issuecomment-2098973037 Could you fix Python linter failure, @grundprinzip ? ``` ./python/pyspark/sql/tests/connect/test_connect_column.py:1029:5: F401 'os' imported but unused import os ^ 1 F401 'os' imported but unused ``` -- 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
