Skip to content

[SPARK-24605][SQL] size(null) returns null instead of -1 #21598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MaxGekk wants to merge 12 commits into from
Closed

[SPARK-24605][SQL] size(null) returns null instead of -1 #21598

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2338b78
New implementation of size returns null for null input
MaxGekk Jun 20, 2018
e18568f
Test for legacy and new implementation of size()
MaxGekk Jun 20, 2018
64dc3b2
Call single param constructor
MaxGekk Jun 20, 2018
b6539a5
Fix expression tests
MaxGekk Jun 20, 2018
80114b3
isPublic flag is reverted back for the global config
MaxGekk Jun 20, 2018
5d58d82
Replacing config name by var name in SQLConf
MaxGekk Jun 20, 2018
431fca8
Folding Array and Map cases to one, throwing exception for unsupporte…
MaxGekk Jun 20, 2018
64a1b7d
Changing config description
MaxGekk Jun 20, 2018
10e7a2d
Updated description and added a couple examples
MaxGekk Jun 26, 2018
cf9364d
Unifying code for legacy and new path
MaxGekk Jun 26, 2018
bb21e13
Merge remote-tracking branch 'origin/master' into size-of-null2
MaxGekk Jun 26, 2018
ae2c7f1
Fix merge conflicts
MaxGekk Jun 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Folding Array and Map cases to one, throwing exception for unsupporte… 
…d operand's type
  • Loading branch information
@MaxGekk
MaxGekk committed Jun 20, 2018
commit 431fca84a045744a9f01e7f731df61c2dcf39fe2
7 changes: 5 additions & 2 deletions ...alyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ case class Size(
} else child.dataType match {
case _: ArrayType => value.asInstanceOf[ArrayData].numElements()
case _: MapType => value.asInstanceOf[MapData].numElements()
case other => throw new UnsupportedOperationException(
s"The size function doesn't support the operand type ${other.getClass.getCanonicalName}")
}
}

Expand All @@ -109,8 +111,9 @@ case class Size(
(${childGen.value}).numElements();""", isNull = FalseLiteral)
} else {
child.dataType match {
case _: ArrayType => defineCodeGen(ctx, ev, c => s"($c).numElements()")
case _: MapType => defineCodeGen(ctx, ev, c => s"($c).numElements()")
case _: ArrayType | _: MapType => defineCodeGen(ctx, ev, c => s"($c).numElements()")
case other => throw new UnsupportedOperationException(
Copy link
Member

@gatorsmile gatorsmile Jun 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you reorganize the code? The current flow looks confusing. It appears like that the other types are not supported when legacySizeOfNull is false. However, the input types are already limited to ArrayType and MapType.

s"The size function doesn't support the operand type ${other.getClass.getCanonicalName}")
}
}
}
Expand Down