RussellSpitzer commented on code in PR #10774:
URL: https://github.com/apache/iceberg/pull/10774#discussion_r1698622883
##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/IcebergSqlExtensionsAstBuilder.scala:
##########
@@ -227,8 +227,10 @@ class IcebergSqlExtensionsAstBuilder(delegate:
ParserInterface) extends IcebergS
val distributionMode = if (distributionSpec != null) {
DistributionMode.HASH
- } else if (orderingSpec.UNORDERED != null || orderingSpec.LOCALLY != null)
{
+ } else if (orderingSpec.UNORDERED != null) {
DistributionMode.NONE
+ } else if (orderingSpec.LOCALLY() != null) {
+ null
Review Comment:
This is a bit confusing to me since I don't think we have it well defined in
SetWriteDistributionAndOrdering that null means don't change. I'm also
generally against using null if possible in Scala.
I think this may require refactoring the SetWriteDistributionAndOrdering to
be a little more flexible? It could take Option(DistributionMode),
Option(SortOrder)?
--
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]
