Skip to content

[SPARK-25371][SQL] struct() should allow being called with 0 args #22373

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
mgaido91 wants to merge 3 commits into from
Closed

[SPARK-25371][SQL] struct() should allow being called with 0 args #22373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7bc026e
[SPARK-25371][SQL] VectorAssembler shpuld not fail with empty inputCols
mgaido91 Sep 9, 2018
41bd5bd
Revert check in struct for more than one argument
mgaido91 Sep 10, 2018
69ff3cb
fix imports
mgaido91 Sep 10, 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
5 changes: 5 additions & 0 deletions mllib/src/test/scala/org/apache/spark/ml/feature/VectorAssemblerSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,9 @@ class VectorAssemblerSuite
assert(runWithMetadata("keep", additional_filter = "id1 > 2").count() == 4)
}

test("SPARK-25371: VectorAssembler with empty inputCols") {
Copy link
Contributor

@cloud-fan cloud-fan Sep 10, 2018

Choose a reason for hiding this comment

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

Do we still need this test?

Copy link
Contributor Author

@mgaido91 mgaido91 Sep 10, 2018

Choose a reason for hiding this comment

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

I think so, after this patch this test passes, before it doesn't. I think it is helpful to avoid regressions like this in the future.

val vectorAssembler = new VectorAssembler().setInputCols(Array()).setOutputCol("a")
Copy link
Member

@dongjoon-hyun dongjoon-hyun Sep 10, 2018

Choose a reason for hiding this comment

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

Is VectorAssembler with zero input column useful?

Copy link
Member

@dongjoon-hyun dongjoon-hyun Sep 10, 2018

Choose a reason for hiding this comment

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

cc @yanboliang and @srowen

Copy link
Member

@srowen srowen Sep 10, 2018

Choose a reason for hiding this comment

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

It doesn't sound that useful, but the JIRA suggests this is the behavior in 2.2. It throws a weird error in 2.3. I could imagine just allowing this behavior, or throwing a better exception. Is there a use case for no input? maybe you have some reusable pipeline that is applied to a subset of columns and sometimes it matches nothing. The output is empty but maybe that doesn't matter for whatever purpose it serves... maybe it's assembled with something else afterwards. I could picture a valid use case.

val output = vectorAssembler.transform(dfWithNullsAndNaNs)
assert(output.select("a").limit(1).collect().head == Row(Vectors.sparse(0, Seq.empty)))
}
}
5 changes: 1 addition & 4 deletions ...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,10 +379,7 @@ trait CreateNamedStructLike extends Expression {
}

override def checkInputDataTypes(): TypeCheckResult = {
if (children.length < 1) {
TypeCheckResult.TypeCheckFailure(
s"input to function $prettyName requires at least one argument")
} else if (children.size % 2 != 0) {
if (children.size % 2 != 0) {
TypeCheckResult.TypeCheckFailure(s"$prettyName expects an even number of arguments.")
} else {
val invalidNames = nameExprs.filterNot(e => e.foldable && e.dataType == StringType)
Expand Down
2 changes: 0 additions & 2 deletions sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2677,8 +2677,6 @@ class DataFrameFunctionsSuite extends QueryTest with SharedSQLContext {
val funcsMustHaveAtLeastOneArg =
("coalesce", (df: DataFrame) => df.select(coalesce())) ::
("coalesce", (df: DataFrame) => df.selectExpr("coalesce()")) ::
("named_struct", (df: DataFrame) => df.select(struct())) ::
("named_struct", (df: DataFrame) => df.selectExpr("named_struct()")) ::
("hash", (df: DataFrame) => df.select(hash())) ::
("hash", (df: DataFrame) => df.selectExpr("hash()")) :: Nil
funcsMustHaveAtLeastOneArg.foreach { case (name, func) =>
Expand Down