dtenedor commented on code in PR #41820:
URL: https://github.com/apache/spark/pull/41820#discussion_r1253357768
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala:
##########
@@ -467,4 +467,15 @@ object ResolveDefaultColumns extends QueryErrorsBase {
v1Catalog.isPersistentFunction(ident.asFunctionIdentifier)
}
}
+
+
Review Comment:
remove extra newline?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala:
##########
@@ -467,4 +467,15 @@ object ResolveDefaultColumns extends QueryErrorsBase {
v1Catalog.isPersistentFunction(ident.asFunctionIdentifier)
}
}
+
+
+ /**
+ * These define and cache existence default values for the struct fields for
efficiency purposes.
Review Comment:
This does not appear to cache the results of each method, as this comment
suggests. The previous code did this with a `lazy val`. Can you update the
comment to mention that these results are *not* cached and that callers should
take care to avoid calling them repeatedly in tight loops during query
execution (e.g. per-row) and instead prefer to initialize them as infrequently
as possible, preferably only once?
--
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]
