You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2023/03/11 08:58:47 UTC
[GitHub] [spark] zhengruifeng opened a new pull request, #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
zhengruifeng opened a new pull request, #40376:
URL: https://github.com/apache/spark/pull/40376
### What changes were proposed in this pull request?
Helper function to convert proto literal to value in Python Client
### Why are the changes needed?
needed in .ml
### Does this PR introduce _any_ user-facing change?
no, dev-only
### How was this patch tested?
added ut
--
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
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40376:
URL: https://github.com/apache/spark/pull/40376#discussion_r1133373330
##########
python/pyspark/sql/connect/expressions.py:
##########
@@ -308,6 +308,43 @@ def _infer_type(cls, value: Any) -> DataType:
def _from_value(cls, value: Any) -> "LiteralExpression":
return LiteralExpression(value=value, dataType=LiteralExpression._infer_type(value))
+ @classmethod
+ def _to_value(cls, literal: "proto.Expression.Literal") -> Any:
+ if literal.HasField("null"):
+ return None
+ elif literal.HasField("binary"):
+ return literal.binary
+ elif literal.HasField("boolean"):
+ return literal.boolean
+ elif literal.HasField("byte"):
+ return literal.byte
+ elif literal.HasField("short"):
+ return literal.short
+ elif literal.HasField("integer"):
+ return literal.integer
+ elif literal.HasField("long"):
+ return literal.long
+ elif literal.HasField("float"):
+ return literal.float
+ elif literal.HasField("double"):
+ return literal.double
+ elif literal.HasField("decimal"):
+ return decimal.Decimal(literal.decimal.value)
+ elif literal.HasField("string"):
+ return literal.string
+ elif literal.HasField("date"):
+ return DateType().fromInternal(literal.date)
+ elif literal.HasField("timestamp"):
+ return TimestampType().fromInternal(literal.timestamp)
+ elif literal.HasField("timestamp_ntz"):
+ return TimestampNTZType().fromInternal(literal.timestamp_ntz)
+ elif literal.HasField("day_time_interval"):
+ return DayTimeIntervalType().fromInternal(literal.day_time_interval)
+ elif literal.HasField("array"):
+ return [LiteralExpression._to_value(v) for v in literal.array.elements]
Review Comment:
let me add one
--
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
[GitHub] [spark] WeichenXu123 commented on a diff in pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "WeichenXu123 (via GitHub)" <gi...@apache.org>.
WeichenXu123 commented on code in PR #40376:
URL: https://github.com/apache/spark/pull/40376#discussion_r1133366703
##########
python/pyspark/sql/connect/expressions.py:
##########
@@ -308,6 +308,43 @@ def _infer_type(cls, value: Any) -> DataType:
def _from_value(cls, value: Any) -> "LiteralExpression":
return LiteralExpression(value=value, dataType=LiteralExpression._infer_type(value))
+ @classmethod
+ def _to_value(cls, literal: "proto.Expression.Literal") -> Any:
+ if literal.HasField("null"):
+ return None
Review Comment:
Shall we raise error in this case ?
##########
python/pyspark/sql/connect/expressions.py:
##########
@@ -308,6 +308,43 @@ def _infer_type(cls, value: Any) -> DataType:
def _from_value(cls, value: Any) -> "LiteralExpression":
return LiteralExpression(value=value, dataType=LiteralExpression._infer_type(value))
+ @classmethod
+ def _to_value(cls, literal: "proto.Expression.Literal") -> Any:
+ if literal.HasField("null"):
+ return None
+ elif literal.HasField("binary"):
+ return literal.binary
+ elif literal.HasField("boolean"):
+ return literal.boolean
+ elif literal.HasField("byte"):
+ return literal.byte
+ elif literal.HasField("short"):
+ return literal.short
+ elif literal.HasField("integer"):
+ return literal.integer
+ elif literal.HasField("long"):
+ return literal.long
+ elif literal.HasField("float"):
+ return literal.float
+ elif literal.HasField("double"):
+ return literal.double
+ elif literal.HasField("decimal"):
+ return decimal.Decimal(literal.decimal.value)
+ elif literal.HasField("string"):
+ return literal.string
+ elif literal.HasField("date"):
+ return DateType().fromInternal(literal.date)
+ elif literal.HasField("timestamp"):
+ return TimestampType().fromInternal(literal.timestamp)
+ elif literal.HasField("timestamp_ntz"):
+ return TimestampNTZType().fromInternal(literal.timestamp_ntz)
+ elif literal.HasField("day_time_interval"):
+ return DayTimeIntervalType().fromInternal(literal.day_time_interval)
+ elif literal.HasField("array"):
+ return [LiteralExpression._to_value(v) for v in literal.array.elements]
Review Comment:
Q: shall we verify the element_type field ?
--
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
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40376:
URL: https://github.com/apache/spark/pull/40376#discussion_r1133369822
##########
python/pyspark/sql/connect/expressions.py:
##########
@@ -308,6 +308,43 @@ def _infer_type(cls, value: Any) -> DataType:
def _from_value(cls, value: Any) -> "LiteralExpression":
return LiteralExpression(value=value, dataType=LiteralExpression._infer_type(value))
+ @classmethod
+ def _to_value(cls, literal: "proto.Expression.Literal") -> Any:
+ if literal.HasField("null"):
+ return None
Review Comment:
`None` here is a valid 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: 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
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40376:
URL: https://github.com/apache/spark/pull/40376#discussion_r1133369721
##########
python/pyspark/sql/connect/expressions.py:
##########
@@ -308,6 +308,43 @@ def _infer_type(cls, value: Any) -> DataType:
def _from_value(cls, value: Any) -> "LiteralExpression":
return LiteralExpression(value=value, dataType=LiteralExpression._infer_type(value))
+ @classmethod
+ def _to_value(cls, literal: "proto.Expression.Literal") -> Any:
+ if literal.HasField("null"):
+ return None
+ elif literal.HasField("binary"):
+ return literal.binary
+ elif literal.HasField("boolean"):
+ return literal.boolean
+ elif literal.HasField("byte"):
+ return literal.byte
+ elif literal.HasField("short"):
+ return literal.short
+ elif literal.HasField("integer"):
+ return literal.integer
+ elif literal.HasField("long"):
+ return literal.long
+ elif literal.HasField("float"):
+ return literal.float
+ elif literal.HasField("double"):
+ return literal.double
+ elif literal.HasField("decimal"):
+ return decimal.Decimal(literal.decimal.value)
+ elif literal.HasField("string"):
+ return literal.string
+ elif literal.HasField("date"):
+ return DateType().fromInternal(literal.date)
+ elif literal.HasField("timestamp"):
+ return TimestampType().fromInternal(literal.timestamp)
+ elif literal.HasField("timestamp_ntz"):
+ return TimestampNTZType().fromInternal(literal.timestamp_ntz)
+ elif literal.HasField("day_time_interval"):
+ return DayTimeIntervalType().fromInternal(literal.day_time_interval)
+ elif literal.HasField("array"):
+ return [LiteralExpression._to_value(v) for v in literal.array.elements]
Review Comment:
that is fine
--
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
[GitHub] [spark] xinrong-meng commented on pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #40376:
URL: https://github.com/apache/spark/pull/40376#issuecomment-1465422914
Do we intentionally ignore `YearMonthIntervalType`?
--
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
[GitHub] [spark] zhengruifeng commented on pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40376:
URL: https://github.com/apache/spark/pull/40376#issuecomment-1465585755
thank you all, merged to master/branch-3.4
--
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
[GitHub] [spark] zhengruifeng commented on pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40376:
URL: https://github.com/apache/spark/pull/40376#issuecomment-1464865482
cc @WeichenXu123 @HyukjinKwon
--
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
[GitHub] [spark] zhengruifeng closed pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
URL: https://github.com/apache/spark/pull/40376
--
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
[GitHub] [spark] zhengruifeng commented on pull request #40376: [SPARK-42756][CONNECT][PYTHON] Helper function to convert proto literal to value in Python Client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40376:
URL: https://github.com/apache/spark/pull/40376#issuecomment-1465436796
> Do we intentionally ignore `YearMonthIntervalType`?
it seems that we have not support `YearMonthIntervalType` in vanilla PySpark, and the Python Client is reusing PySpark's types.
So it seems that we should add `YearMonthIntervalType` in PySpark first
--
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