ulysses-you commented on code in PR #57181:
URL: https://github.com/apache/spark/pull/57181#discussion_r3584107829
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/LiteralExpressionSuite.scala:
##########
@@ -855,4 +855,48 @@ class LiteralExpressionSuite extends SparkFunSuite with
ExpressionEvalHelper {
assert(Literal(UTF8String.fromString("x"), StringType("UTF8_LCASE")).sql
===
"'x' collate UTF8_LCASE")
}
+
+ test("valueSizeInBytes") {
+ // A null value reports the type's default size.
+ assert(Literal.create(null, StringType).valueSizeInBytes ===
Some(StringType.defaultSize))
+ assert(Literal.create(null, IntegerType).valueSizeInBytes ===
Some(IntegerType.defaultSize))
+
+ // Fixed-length types report their default size.
+ assert(Literal(1).valueSizeInBytes === Some(IntegerType.defaultSize))
+ assert(Literal(1L).valueSizeInBytes === Some(LongType.defaultSize))
+ assert(Literal(1.0).valueSizeInBytes === Some(DoubleType.defaultSize))
+ assert(Literal(true).valueSizeInBytes === Some(BooleanType.defaultSize))
+ assert(Literal(Decimal(1), DecimalType(10, 0)).valueSizeInBytes ===
+ Some(DecimalType(10, 0).defaultSize))
+
+ // Variable-length string / binary report their real byte length, not the
default size.
+ assert(Literal(UTF8String.fromString(""), StringType).valueSizeInBytes ===
Some(0))
+ assert(Literal(UTF8String.fromString("abc"), StringType).valueSizeInBytes
=== Some(3))
+ // A multi-byte UTF-8 character counts its encoded bytes (U+00E9 encodes
to 2 bytes).
+ assert(Literal(UTF8String.fromString("\u00e9"),
StringType).valueSizeInBytes === Some(2))
Review Comment:
Fixed in a40f1d68a1f. The character is now built with `new
String(Character.toChars(0xE9))` instead of an escape, so scalariform has
nothing to decode and `NonASCIICharacterChecker` sees only ASCII. Ran
`catalyst/scalastyle` and `catalyst/Test/scalastyle` -- 0 errors -- and the
`valueSizeInBytes` test still passes.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/DemoteBroadcastHashJoin.scala:
##########
@@ -21,21 +21,15 @@ import org.apache.spark.MapOutputStatistics
import org.apache.spark.sql.catalyst.optimizer.JoinSelectionHelper
import org.apache.spark.sql.catalyst.planning.ExtractEquiJoinKeys
import org.apache.spark.sql.catalyst.plans.{LeftAnti, LeftOuter, RightOuter}
-import org.apache.spark.sql.catalyst.plans.logical.{HintInfo, Join,
JoinStrategyHint, LogicalPlan, NO_BROADCAST_HASH, PREFER_SHUFFLE_HASH,
SHUFFLE_HASH}
+import org.apache.spark.sql.catalyst.plans.logical.{HintInfo, Join,
JoinStrategyHint, LogicalPlan, NO_BROADCAST_HASH}
import org.apache.spark.sql.catalyst.rules.Rule
-import org.apache.spark.sql.internal.SQLConf
/**
- * This optimization rule includes three join selection:
- * 1. detects a join child that has a high ratio of empty partitions and
adds a
- * NO_BROADCAST_HASH hint to avoid it being broadcast, as shuffle join is
faster in this case:
- * many tasks complete immediately since one join side is empty.
- * 2. detects a join child that every partition size is less than local map
threshold and adds a
- * PREFER_SHUFFLE_HASH hint to encourage being shuffle hash join instead
of sort merge join.
- * 3. if a join satisfies both NO_BROADCAST_HASH and PREFER_SHUFFLE_HASH,
- * then add a SHUFFLE_HASH hint.
+ * This optimization rule detects a join child that has a high ratio of empty
partitions and adds a
+ * NO_BROADCAST_HASH hint to avoid it being broadcast, as shuffle join is
faster in this case: many
+ * tasks complete immediately since one join side is empty.
*/
-object DynamicJoinSelection extends Rule[LogicalPlan] with JoinSelectionHelper
{
+object DemoteBroadcastHashJoin extends Rule[LogicalPlan] with
JoinSelectionHelper {
Review Comment:
Added in a40f1d68a1f, under "Upgrading from Spark SQL 4.2 to 4.3". The entry
covers both the rename (`DynamicJoinSelection` -> `DemoteBroadcastHashJoin`,
now demote-only) and that the SHJ-over-SMJ selection moved to the new physical
rule; it also tells users who excluded `DynamicJoinSelection` via
`spark.sql.adaptive.optimizer.excludedRules` (now a silently-ignored unknown
name) to set
`spark.sql.adaptive.convertSortMergeJoinToShuffledHashJoin.enabled=false`
instead.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/joins.scala:
##########
@@ -287,7 +288,7 @@ case object BuildRight extends BuildSide
case object BuildLeft extends BuildSide
-trait JoinSelectionHelper extends Logging {
+trait JoinSelectionHelper extends SQLConfHelper with Logging {
Review Comment:
Reverted in a40f1d68a1f -- `JoinSelectionHelper` is back to `extends
Logging` and the now-unused `SQLConfHelper` import is removed. The physical
rule still gets `conf` from `Rule` (which itself extends `SQLConfHelper`), and
every method in the trait takes `conf: SQLConf` explicitly, so nothing relied
on the mixin. Both modules still compile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
