holdenk commented on code in PR #59060:
URL: https://github.com/apache/spark/pull/59060#discussion_r4138132516


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -649,6 +649,21 @@ object SQLConf {
     .stringConf
     .createWithDefault("catalyst")
 
+  val PYTHON_UDF_TRANSPILE_NULL_STRICTNESS =
+    buildConf("spark.sql.experimental.optimizer.transpileNullStrictness")
+    .withBindingPolicy(ConfigBindingPolicy.SESSION)
+    .doc("Controls how the Python UDF transpiler handles null/None semantics 
for ordering " +
+      "comparisons (``<``, ``<=``, ``>``, ``>=``). Under ``strict`` (the 
default) a NULL " +
+      "operand causes the expression to raise an error, matching Python's 
TypeError. Under " +
+      "``loose`` a NULL operand silently propagates as NULL instead of 
raising, which allows " +
+      "more UDFs to be transpiled at the cost of minor semantic divergence on 
NULL inputs.")
+    .version("4.3.0")

Review Comment:
   Lets target 4.4, 4.3 RC is going to be cut soon so not going to land 
anything besides bug fixes in 4.3 now



##########
python/pyspark/sql/transpile.py:
##########
@@ -238,7 +241,8 @@ class CatalystTranspiler(AbstractTranspiler):
 
     variety = "catalyst"
 
-    def __init__(self) -> None:
+    def __init__(self, null_strict: bool = True) -> None:
+        self._null_strict = null_strict

Review Comment:
   Maybe just set based on config v.s. as a param? I know it makes testing a 
little more akward though so I'm open to push back.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -649,6 +649,21 @@ object SQLConf {
     .stringConf
     .createWithDefault("catalyst")
 
+  val PYTHON_UDF_TRANSPILE_NULL_STRICTNESS =
+    buildConf("spark.sql.experimental.optimizer.transpileNullStrictness")
+    .withBindingPolicy(ConfigBindingPolicy.SESSION)
+    .doc("Controls how the Python UDF transpiler handles null/None semantics 
for ordering " +
+      "comparisons (``<``, ``<=``, ``>``, ``>=``). Under ``strict`` (the 
default) a NULL " +
+      "operand causes the expression to raise an error, matching Python's 
TypeError. Under " +
+      "``loose`` a NULL operand silently propagates as NULL instead of 
raising, which allows " +
+      "more UDFs to be transpiled at the cost of minor semantic divergence on 
NULL inputs.")
+    .version("4.3.0")
+    .stringConf
+    .checkValue(
+      v => v == "strict" || v == "loose",
+      "Invalid value. Must be 'strict' or 'loose'.")
+    .createWithDefault("strict")

Review Comment:
   I like the naming, but since it's a string type we have to check the string 
instead of just using the boolean value.



-- 
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