You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "itholic (via GitHub)" <gi...@apache.org> on 2024/09/11 11:07:55 UTC

[PR] [SPARK-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

itholic opened a new pull request, #48074:
URL: https://github.com/apache/spark/pull/48074

   ### What changes were proposed in this pull request?
   
   This PR proposes to support non-column arguments in UDTF
   
   ### Why are the changes needed?
   
   Simplifies UDTF usage by removing the need for explicit column conversions.
   
   ### Does this PR introduce _any_ user-facing change?
   
   It does not break the existing behavior, but users can now also pass non-Column arguments directly to UDTFs.
   
   For example, let's say we have an simple UDTF `RepeatItem` as below:
   
   ```python
   from pyspark.sql.functions import udtf, lit
   from pyspark.sql.types import StructType, IntegerType
   
   @udtf(returnType="item: int")
   class RepeatItem:
       """
       A UDTF that returns `item` as many rows as specified in `num_rows`.
       """
       def eval(self, item: int, num_rows: int):
           for i in range(0, num_rows):
               yield (item,)
   ```
   
   UDTF allows direct input without `lit()`:
   
   **Before**
   
   ```python
   RepeatItem(lit(2024), lit(6)).show()
   ```
   
   **After** (NOTE: Before also still works)
   ```
   RepeatItem(2024, 6).show()
   ```
   
   
   ### How was this patch tested?
   
   Added UTs for the existing test suites
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No


-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic closed pull request #48074: [SPARK-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage
URL: https://github.com/apache/spark/pull/48074


-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48074:
URL: https://github.com/apache/spark/pull/48074#discussion_r1756031195


##########
examples/src/main/python/sql/udtf.py:
##########
@@ -39,13 +39,13 @@ def eval(self, start: int, end: int):
             for num in range(start, end + 1):
                 yield (num, num * num)
 
-    from pyspark.sql.functions import lit, udtf
+    from pyspark.sql.functions import udtf
 
     # Create a UDTF using the class definition and the `udtf` function.
     square_num = udtf(SquareNumbers, returnType="num: int, squared: int")
 
     # Invoke the UDTF in PySpark.
-    square_num(lit(1), lit(3)).show()
+    square_num(1, 3).show()

Review Comment:
   +1, the string input might be tricky to decide



-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "ueshin (via GitHub)" <gi...@apache.org>.
ueshin commented on PR #48074:
URL: https://github.com/apache/spark/pull/48074#issuecomment-2347002340

   > Then what about we only apply `lit` for non-string type args and keep the string type args as it is??
   
   IMO, if it accepts non-string type args, it should also accept string type args; otherwise it shouldn't accept any literals, to be consistent.


-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #48074:
URL: https://github.com/apache/spark/pull/48074#issuecomment-2345060205

   @zhengruifeng Thanks for the example!
   
   Then what about we only apply `lit` for non-string type argument and keep the string args as it 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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #48074:
URL: https://github.com/apache/spark/pull/48074#discussion_r1755241144


##########
python/pyspark/sql/udtf.py:
##########
@@ -376,10 +376,13 @@ def __call__(self, *args: "ColumnOrName", **kwargs: "ColumnOrName") -> "DataFram
         from pyspark.sql.classic.column import _to_java_column, _to_seq
 
         from pyspark.sql import DataFrame, SparkSession
+        from pyspark.sql.functions import lit
 
         spark = SparkSession._getActiveSessionOrCreate()
         sc = spark.sparkContext
 
+        args = tuple(lit(arg) for arg in args)

Review Comment:
   Hmm we can support column references with lateral joins for Python UDTFs in the future, so the `arg` input here might not always be literals (e.g. `df.join(MyUDTF("c1", "c2"), ...)`  where "c1" and "c2" are column references. 



-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #48074:
URL: https://github.com/apache/spark/pull/48074#issuecomment-2345053140

   I think we should keep in line with UDF in this case:
   ```
   In [5]: slen = udf(lambda s: len(s), IntegerType())
   
   In [6]: add_one = udf(lambda s: s + 1, IntegerType())
   
   In [7]: spark.range(10).select(add_one("id")).show()
   +------------+
   |<lambda>(id)|
   +------------+
   |           1|
   |           2|
   |           3|
   |           4|
   |           5|
   |           6|
   |           7|
   |           8|
   |           9|
   |          10|
   +------------+
   
   
   In [8]: spark.range(10).select(add_one(1)).show()
   ---------------------------------------------------------------------------
   PySparkTypeError                          Traceback (most recent call last)
   Cell In[8], line 1
   ----> 1 spark.range(10).select(add_one(1)).show()
   
   File ~/Dev/spark/python/pyspark/sql/udf.py:495, in UserDefinedFunction._wrapped.<locals>.wrapper(*args, **kwargs)
       493 @functools.wraps(self.func, assigned=assignments)
       494 def wrapper(*args: "ColumnOrName", **kwargs: "ColumnOrName") -> Column:
   --> 495     return self(*args, **kwargs)
   
   File ~/Dev/spark/python/pyspark/sql/udf.py:405, in UserDefinedFunction.__call__(self, *args, **kwargs)
       402 sc = get_active_spark_context()
       404 assert sc._jvm is not None
   --> 405 jcols = [_to_java_column(arg) for arg in args] + [
       406     sc._jvm.PythonSQLUtils.namedArgumentExpression(key, _to_java_column(value))
       407     for key, value in kwargs.items()
       408 ]
       410 profiler_enabled = sc._conf.get("spark.python.profile", "false") == "true"
       411 memory_profiler_enabled = sc._conf.get("spark.python.profile.memory", "false") == "true"
   
   File ~/Dev/spark/python/pyspark/sql/classic/column.py:71, in _to_java_column(col)
        69     jcol = _create_column_from_name(col)
        70 else:
   ---> 71     raise PySparkTypeError(
        72         errorClass="NOT_COLUMN_OR_STR",
        73         messageParameters={"arg_name": "col", "arg_type": type(col).__name__},
        74     )
        75 return jcol
   
   PySparkTypeError: [NOT_COLUMN_OR_STR] Argument `col` should be a Column or str, got int.
   ```
   
   Python UDF treat `str` argument as column name, and it doesn't accept other non-column arguments


-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #48074:
URL: https://github.com/apache/spark/pull/48074#issuecomment-2345720486

   > @zhengruifeng Thanks for the example!
   > 
   > Then what about we only apply `lit` for non-string type args and keep the string type args as it is??
   
   I am not sure about it, cc @ueshin @HyukjinKwon WDYT?


-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48074:
URL: https://github.com/apache/spark/pull/48074#discussion_r1755906173


##########
python/pyspark/sql/udtf.py:
##########
@@ -376,10 +376,13 @@ def __call__(self, *args: "ColumnOrName", **kwargs: "ColumnOrName") -> "DataFram
         from pyspark.sql.classic.column import _to_java_column, _to_seq
 
         from pyspark.sql import DataFrame, SparkSession
+        from pyspark.sql.functions import lit
 
         spark = SparkSession._getActiveSessionOrCreate()
         sc = spark.sparkContext
 
+        args = tuple(lit(arg) for arg in args)

Review Comment:
   I see. Then I wonder is there any use case using string args that we can add a test suite for now? 



-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48074:
URL: https://github.com/apache/spark/pull/48074#discussion_r1754186287


##########
examples/src/main/python/sql/udtf.py:
##########
@@ -39,13 +39,13 @@ def eval(self, start: int, end: int):
             for num in range(start, end + 1):
                 yield (num, num * num)
 
-    from pyspark.sql.functions import lit, udtf
+    from pyspark.sql.functions import udtf
 
     # Create a UDTF using the class definition and the `udtf` function.
     square_num = udtf(SquareNumbers, returnType="num: int, squared: int")
 
     # Invoke the UDTF in PySpark.
-    square_num(lit(1), lit(3)).show()
+    square_num(1, 3).show()

Review Comment:
   dumb question: in case a input is `str`, e.g.
   
   ```
   square_num(`a`, 3)
   ```
   
   should we treat `a` as a literal string or a column name (like most functions) ?
   



-- 
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-49597][PYTHON][CONNECT] Support non-column arguments in UDTF for simpler usage [spark]

Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #48074:
URL: https://github.com/apache/spark/pull/48074#issuecomment-2343334348

   cc @allisonwang-db @ueshin 


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