Skip to content
Closed
Next Next commit
Canonicalization on GetStructField
  • Loading branch information
dbtsai committed Dec 19, 2018
commit 43351dfabb33fb338b3904dafa4c91bdac58ee5a
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ object Canonicalize {
private[expressions] def ignoreNamesTypes(e: Expression): Expression = e match {
case a: AttributeReference =>
AttributeReference("none", a.dataType.asNullable)(exprId = a.exprId)
case GetStructField(child, ordinal, _) => GetStructField(child, ordinal, None)
Copy link
Member

Choose a reason for hiding this comment

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

+1 for this canonicalization.

case _ => e
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package org.apache.spark.sql.catalyst.expressions
import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.dsl.plans._
import org.apache.spark.sql.catalyst.plans.logical.Range
import org.apache.spark.sql.types.{IntegerType, StructField, StructType}

class CanonicalizeSuite extends SparkFunSuite {

Expand Down Expand Up @@ -50,4 +51,31 @@ class CanonicalizeSuite extends SparkFunSuite {
assert(range.where(arrays1).sameResult(range.where(arrays2)))
assert(!range.where(arrays1).sameResult(range.where(arrays3)))
}

test("SPARK-26402: GetStructField with different optional names are semantically equal") {
val structType =
StructType(StructField("a", StructType(StructField("b", IntegerType) :: Nil)) :: Nil)
val expId = NamedExpression.newExprId
val qualifier = Seq.empty[String]

val fieldB1 = GetStructField(
AttributeReference("data1", structType, true)(expId, qualifier),
0, Some("b1"))
Copy link
Member

@dongjoon-hyun dongjoon-hyun Dec 21, 2018

Choose a reason for hiding this comment

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

Sorry for nit-picking. This should be a1 (and a2 at line 67) because this is the first level.
Consequently, fieldB1 -> fieldA1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Done.

Copy link
Member

Choose a reason for hiding this comment

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

    val fieldA1 = GetStructField(
      AttributeReference("data1", structType, false)(expId, qualifier),
      0, Some("a1"))
    val fieldA2 = GetStructField(
      AttributeReference("data2", structType, false)(expId, qualifier),
      0, Some("a2"))
    assert(fieldA1.semanticEquals(fieldA2))

    val fieldB1 = GetStructField(
      GetStructField(
        AttributeReference("data1", structType, false)(expId, qualifier),
        0, Some("a1")),
      0, Some("b1"))
    val fieldB2 = GetStructField(
      GetStructField(
        AttributeReference("data2", structType, false)(expId, qualifier),
        0, Some("a2")),
      0, Some("b2"))
    assert(fieldB1.semanticEquals(fieldB2))

Copy link
Member Author

Choose a reason for hiding this comment

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

@dongjoon-hyun I put the ordering wrong. Addressed as you suggested. Thanks!

val fieldB2 = GetStructField(
AttributeReference("data2", structType, true)(expId, qualifier),
0, Some("b2"))
assert(fieldB1.semanticEquals(fieldB2))
Copy link
Member

Choose a reason for hiding this comment

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

This line will fail to build.


val fieldA1 = GetStructField(
GetStructField(
AttributeReference("data1", structType, true)(expId, qualifier),
0, Some("a1")),
0, Some("b1"))
val fieldA2 = GetStructField(
GetStructField(
AttributeReference("data2", structType, true)(expId, qualifier),
0, Some("a2")),
0, Some("b2"))
assert(fieldA1.semanticEquals(fieldA2))
}
}