singhpk234 commented on code in PR #51002:
URL: https://github.com/apache/spark/pull/51002#discussion_r2114168866
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala:
##########
@@ -597,11 +597,31 @@ private[sql] object CatalogV2Util {
// Note: the back-fill here is a logical concept. The data source can
keep the existing
// data unchanged and let the data reader to return "exist
default" for missing
// columns.
- val existingDefault = Literal(default.getValue.value(),
default.getValue.dataType()).sql
-
f.withExistenceDefaultValue(existingDefault).withCurrentDefaultValue(default.getSql)
+ val existsDefault = extractExistsDefault(default)
+ val (sql, expr) = extractCurrentDefault(default)
+ val newMetadata = new MetadataBuilder()
+ .withMetadata(f.metadata)
+ .putString(EXISTS_DEFAULT_COLUMN_METADATA_KEY, existsDefault)
+ .putExpression(CURRENT_DEFAULT_COLUMN_METADATA_KEY, sql, expr)
+ .build()
+ f.copy(metadata = newMetadata)
}.getOrElse(f)
}
+ private def extractExistsDefault(default: ColumnDefaultValue): String = {
+ Literal(default.getValue.value(), default.getValue.dataType()).sql
+ }
+
+ private def extractCurrentDefault(default: ColumnDefaultValue): (String,
Option[Expression]) = {
+ val expr =
Option(default.getExpression).flatMap(V2ExpressionUtils.toCatalyst)
Review Comment:
[doubt] presently `toCatalyst` doesn't handle connector scalar udf's is the
plan to enhance this in future ?
##########
sql/api/src/main/scala/org/apache/spark/sql/types/Metadata.scala:
##########
@@ -120,6 +122,12 @@ sealed class Metadata private[types] (private[types] val
map: Map[String, Any])
map(key).asInstanceOf[T]
}
+ private[sql] def getExpression[E](key: String): (String, Option[E]) = {
+ val sql = getString(key)
+ val expr = if (runtimeMap != null)
runtimeMap.get(key).map(_.asInstanceOf[E]) else None
Review Comment:
minor
```suggestion
val expr = Option(runtimeMap).flatMap(_.get(key).map(_.asInstanceOf[E]))
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala:
##########
@@ -597,11 +597,31 @@ private[sql] object CatalogV2Util {
// Note: the back-fill here is a logical concept. The data source can
keep the existing
// data unchanged and let the data reader to return "exist
default" for missing
// columns.
- val existingDefault = Literal(default.getValue.value(),
default.getValue.dataType()).sql
-
f.withExistenceDefaultValue(existingDefault).withCurrentDefaultValue(default.getSql)
+ val existsDefault = extractExistsDefault(default)
+ val (sql, expr) = extractCurrentDefault(default)
+ val newMetadata = new MetadataBuilder()
+ .withMetadata(f.metadata)
+ .putString(EXISTS_DEFAULT_COLUMN_METADATA_KEY, existsDefault)
+ .putExpression(CURRENT_DEFAULT_COLUMN_METADATA_KEY, sql, expr)
+ .build()
+ f.copy(metadata = newMetadata)
}.getOrElse(f)
}
+ private def extractExistsDefault(default: ColumnDefaultValue): String = {
+ Literal(default.getValue.value(), default.getValue.dataType()).sql
+ }
+
+ private def extractCurrentDefault(default: ColumnDefaultValue): (String,
Option[Expression]) = {
+ val expr =
Option(default.getExpression).flatMap(V2ExpressionUtils.toCatalyst)
+ val sql = Option(default.getSql).orElse(expr.map(_.sql)).getOrElse {
Review Comment:
[doubt] my understanding was `.sql` is not reliable (based on discussion
[here](https://github.com/apache/spark/pull/50792#discussion_r2078776399)),
wondering if this could lead to users using `getMap` or map.get(key), directly
and extracting the SQL from the map, skip actually checking if there is an
expression for it and one should use that instead ? essentially if there i an
entry in the runtimeMap should we let the map.get fail ?
--
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
