-
Notifications
You must be signed in to change notification settings - Fork 75
[NSE-732] Support Struct complex type in Shuffle #733
[NSE-732] Support Struct complex type in Shuffle #733
Conversation
@zhixingheyi-tian this method needs to be improved for Map/Struct |
case d: StructType => | ||
case _ => | ||
try { | ||
ConverterUtils.createArrowField(attr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disable nested array support here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhouyuan Done
@@ -517,6 +516,27 @@ object ConverterUtils extends Logging { | |||
throw new UnsupportedOperationException(s"Unsupported data type: $dt") | |||
} | |||
|
|||
def checkIfNestTypeSupported(dt: DataType): Unit = dt match { | |||
case d: ArrayType => checkIfTypeSupported(d.elementType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array[Array[]] still supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhouyuan
checkIfTypeSupported is another check function, it has excluded the Array/Struct/Map support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rename this like checkPrimitiveTypes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"checkIfTypeSupported" is reuse the existed func. It has been used in too many Operator scala files.
import io.netty.buffer.{ByteBuf, ByteBufAllocator, ByteBufOutputStream} | ||
import java.nio.channels.{Channels, WritableByteChannel} | ||
|
||
import com.google.common.collect.Lists | ||
import java.io.{InputStream, OutputStream} | ||
import java.util |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import java.util.ArrayList ?
related: #461 |
What changes were proposed in this pull request?
Let's focus on Struct support firstly. Because there are many points needed to modify for one new complex type.
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)