You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "chenhao-db (via GitHub)" <gi...@apache.org> on 2024/04/09 05:39:26 UTC
[PR] [SPARK-47775][SQL] Support remaining scalar types in the variant spec. [spark]
chenhao-db opened a new pull request, #45945:
URL: https://github.com/apache/spark/pull/45945
### What changes were proposed in this pull request?
This PR adds support for the remaining scalar types defined in the variant spec (DATE, TIMESTAMP, TIMESTAMP_NTZ, FLOAT, BINARY). The current `parse_json` expression doesn't produce these types, but we need them when we support casting a corresponding Spark type into the variant type.
### Why are the changes needed?
This PR can be considered as a preparation for the cast-to-variant feature and will make the latter PR smaller.
### Does this PR introduce _any_ user-facing change?
Yes. Existing variant expressions can decode more variant scalar types.
### How was this patch tested?
Unit tests. We manually construct variant values with these new scalar types and test the existing variant expressions on them.
### 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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #45945: [SPARK-47775][SQL] Support remaining scalar types in the variant spec.
URL: https://github.com/apache/spark/pull/45945
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1557634726
##########
common/variant/src/main/java/org/apache/spark/types/variant/Variant.java:
##########
@@ -89,6 +97,37 @@ public BigDecimal getDecimal() {
return VariantUtil.getDecimal(value, pos);
}
+ // Get a raw date value (number of days from the Unix epoch) from the variant.
+ public int getRawDate() {
Review Comment:
Similarly, `UnsafeRow` in Spark doesn't have these methods either. The call just call `getInt/getLong` and interpret the result as number of days/seconds from Unix epoch.
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "chenhao-db (via GitHub)" <gi...@apache.org>.
chenhao-db commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1558098749
##########
common/variant/src/main/java/org/apache/spark/types/variant/Variant.java:
##########
@@ -89,6 +97,37 @@ public BigDecimal getDecimal() {
return VariantUtil.getDecimal(value, pos);
}
+ // Get a raw date value (number of days from the Unix epoch) from the variant.
+ public int getRawDate() {
Review Comment:
Thanks for the suggestion. This simplifies the implementation a lot.
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1557628324
##########
common/variant/src/main/java/org/apache/spark/types/variant/Variant.java:
##########
@@ -89,6 +97,37 @@ public BigDecimal getDecimal() {
return VariantUtil.getDecimal(value, pos);
}
+ // Get a raw date value (number of days from the Unix epoch) from the variant.
+ public int getRawDate() {
Review Comment:
do we need to have these methods? date/timestamp are logical types and are physically int/long. We just need the metadata to indicate them.
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1557634726
##########
common/variant/src/main/java/org/apache/spark/types/variant/Variant.java:
##########
@@ -89,6 +97,37 @@ public BigDecimal getDecimal() {
return VariantUtil.getDecimal(value, pos);
}
+ // Get a raw date value (number of days from the Unix epoch) from the variant.
+ public int getRawDate() {
Review Comment:
Similarly, `UnsafeRow` in Spark doesn't have these methods either. The caller just invoke `getInt/getLong` and interpret the result as number of days/seconds from Unix epoch.
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1558932654
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala:
##########
@@ -248,9 +253,10 @@ case object VariantGet {
input: VariantVal,
dataType: DataType,
failOnError: Boolean,
- zoneId: Option[String]): Any = {
+ zoneStr: Option[String],
Review Comment:
why would we need zone id string during execution?
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "chenhao-db (via GitHub)" <gi...@apache.org>.
chenhao-db commented on code in PR #45945:
URL: https://github.com/apache/spark/pull/45945#discussion_r1559545416
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/variant/variantExpressions.scala:
##########
@@ -248,9 +253,10 @@ case object VariantGet {
input: VariantVal,
dataType: DataType,
failOnError: Boolean,
- zoneId: Option[String]): Any = {
+ zoneStr: Option[String],
Review Comment:
The zone id string is needed to construct the `Cast` object. It can also be obtained from `zoneId` (`zoneStr == Some(zoneId.getId)`), but that would require more runtime cost than passing both the string and the id.
--
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-47775][SQL] Support remaining scalar types in the variant spec. [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #45945:
URL: https://github.com/apache/spark/pull/45945#issuecomment-2047765218
thanks, 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-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