-
Notifications
You must be signed in to change notification settings - Fork 28k
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.
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
[SPARK-48296][SQL] Codegen Support for to_xml
#46591
Conversation
@@ -55,7 +55,7 @@ class XmlFunctionsSuite extends QueryTest with SharedSparkSession { | |||
val options = Map("rowTag" -> "foo").asJava | |||
|
|||
checkAnswer( | |||
df.select(from_xml($"value", schema)), | |||
df.select(from_xml($"value", schema, options)), |
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.
Obviously, its usage does not match the title
of UT
to_xml
@@ -186,9 +186,6 @@ case class SchemaOfXml( | |||
@transient | |||
private lazy val xmlOptions = new XmlOptions(options, "UTC") | |||
|
|||
@transient | |||
private lazy val xmlFactory = xmlOptions.buildXmlFactory() |
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.
This variable xmlFactory
is not used in the expression SchemaOfXml
, so it is deleted here. It has no strong relationship with this PR.
|
||
override def inputTypes: Seq[AbstractDataType] = StructType :: Nil | ||
|
||
override def prettyName: String = "to_xml" | ||
|
||
override protected def withNewChildInternal(newChild: Expression): StructsToXml = | ||
copy(child = newChild) | ||
|
||
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { |
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.
Simply
override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val expr = ctx.addReferenceObj("this", this)
defineCodeGen(ctx, ev, input => s"(UTF8String) $expr.nullSafeEval($input)")
}
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.
yeah, updated.
thanks!
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.
We can also revert other refactoring?
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.
let me have a try.
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.
Done.
sql/core/src/test/scala/org/apache/spark/sql/XmlFunctionsSuite.scala
Outdated
Show resolved
Hide resolved
….scala Co-authored-by: Kent Yao <yao@apache.org>
Thanks @panbingkun, merged to master |
What changes were proposed in this pull request?
The PR adds
Codegen Support
forto_xml
.Why are the changes needed?
Improve codegen coverage.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.