Skip to content

Removed accidentally checked in comment - #61

Closed
kayousterhout wants to merge 1 commit into
apache:masterfrom
kayousterhout:remove_comment
Closed

Removed accidentally checked in comment#61
kayousterhout wants to merge 1 commit into
apache:masterfrom
kayousterhout:remove_comment

Conversation

@kayousterhout

Copy link
Copy Markdown
Contributor

It looks like this comment was added a while ago by @mridulm as part of a merge and was accidentally checked in. We should remove it.

@mridulm

Copy link
Copy Markdown
Contributor

Yeah, this was an internal review comment :-)
Thanks !

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12960/

@shivaram

Copy link
Copy Markdown
Contributor

LGTM

@rxin

rxin commented Mar 3, 2014

Copy link
Copy Markdown
Contributor

I merged this. Thanks!

@asfgitasfgit closed this in 369aad6Mar 3, 2014
jhartlaub referenced this pull request in jhartlaub/spark May 27, 2014
Unified daemon thread pools
As requested by @mateiz in an earlier pull request, this refactors various daemon thread pools to use a set of methods in utils.scala, and also changes the thread-pool-creation methods in utils.scala to use named thread pools for improved debugging.
(cherry picked from commit 983b83f)
Signed-off-by: Reynold Xin <rxin@apache.org>
wli600 pushed a commit to wli600/spark that referenced this pull request Jul 29, 2015
JasonMWhite pushed a commit to JasonMWhite/spark that referenced this pull request Dec 2, 2015
jlopezmalla pushed a commit to jlopezmalla/spark that referenced this pull request Sep 18, 2017
* reverted kms download
* Update DockerfileDispatcher
* Update Jenkinsfile
Igosuki pushed a commit to Adikteev/spark that referenced this pull request Jul 31, 2018
su8su pushed a commit to su8su/spark that referenced this pull request Dec 11, 2018
su8su pushed a commit to su8su/spark that referenced this pull request Dec 11, 2018
weixiuli pushed a commit to weixiuli/spark that referenced this pull request Jun 18, 2019
* auto calculate the initial partition number
* update style
* update style and add ut
* use Math.ceil to handle the not divisible situation
* add configuration for this feature and calculate the statistics info of needed column not the table
* update the statistics of partitioned table
* rename parameters
* collect all the leaves node when calculate the initial partition num and some small udate
* small update
hejian991 pushed a commit to growingio/spark that referenced this pull request Jun 24, 2019
* auto calculate the initial partition number
* update style
* update style and add ut
* use Math.ceil to handle the not divisible situation
* add configuration for this feature and calculate the statistics info of needed column not the table
* update the statistics of partitioned table
* rename parameters
* collect all the leaves node when calculate the initial partition num and some small udate
* small update
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
We have so many scenarios about Kubernetes and OpenStack integration,
An unified name format of Ansible jobs is necessary. Add empty
directories for known scenario, and the ansible jobs name should follow
the format.
Partial-issue: theopenlab#27
cloud-fan pushed a commit that referenced this pull request Jan 14, 2021
…join can be planned as broadcast join
### What changes were proposed in this pull request?
Should not pushdown LeftSemi/LeftAnti over Aggregate for some cases.
```scala
spark.range(50000000L).selectExpr("id % 10000 as a", "id % 10000 as b").write.saveAsTable("t1")
spark.range(40000000L).selectExpr("id % 8000 as c", "id % 8000 as d").write.saveAsTable("t2")
spark.sql("SELECT distinct a, b FROM t1 INTERSECT SELECT distinct c, d FROM t2").explain
```
Before this pr:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#72]
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- SortMergeJoin [coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L)], [coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L)], LeftSemi
:- Sort [coalesce(a#16L, 0) ASC NULLS FIRST, isnull(a#16L) ASC NULLS FIRST, coalesce(b#17L, 0) ASC NULLS FIRST, isnull(b#17L) ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L), 5), ENSURE_REQUIREMENTS, [id=#65]
: +- FileScan parquet default.t1[a#16L,b#17L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:bigint,b:bigint>
+- Sort [coalesce(c#18L, 0) ASC NULLS FIRST, isnull(c#18L) ASC NULLS FIRST, coalesce(d#19L, 0) ASC NULLS FIRST, isnull(d#19L) ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L), 5), ENSURE_REQUIREMENTS, [id=#66]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- Exchange hashpartitioning(c#18L, d#19L, 5), ENSURE_REQUIREMENTS, [id=#61]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- FileScan parquet default.t2[c#18L,d#19L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<c:bigint,d:bigint>
```
After this pr:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#74]
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- SortMergeJoin [coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L)], [coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L)], LeftSemi
:- Sort [coalesce(a#16L, 0) ASC NULLS FIRST, isnull(a#16L) ASC NULLS FIRST, coalesce(b#17L, 0) ASC NULLS FIRST, isnull(b#17L) ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L), 5), ENSURE_REQUIREMENTS, [id=#67]
: +- HashAggregate(keys=[a#16L, b#17L], functions=[])
: +- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#61]
: +- HashAggregate(keys=[a#16L, b#17L], functions=[])
: +- FileScan parquet default.t1[a#16L,b#17L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:bigint,b:bigint>
+- Sort [coalesce(c#18L, 0) ASC NULLS FIRST, isnull(c#18L) ASC NULLS FIRST, coalesce(d#19L, 0) ASC NULLS FIRST, isnull(d#19L) ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L), 5), ENSURE_REQUIREMENTS, [id=#68]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- Exchange hashpartitioning(c#18L, d#19L, 5), ENSURE_REQUIREMENTS, [id=#63]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- FileScan parquet default.t2[c#18L,d#19L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<c:bigint,d:bigint>
```
### Why are the changes needed?
1. Pushdown LeftSemi/LeftAnti over Aggregate will affect performance.
2. It will remove user added DISTINCT operator, e.g.: [q38](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q38.sql), [q87](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q87.sql).
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test and benchmark test.
SQL | Before this PR(Seconds) | After this PR(Seconds)
-- | -- | --
q14a | 660 | 594
q14b | 660 | 600
q38 | 55 | 29
q87 | 66 | 35
Before this pr:
![image](https://user-images.githubusercontent.com/5399861/104452849-8789fc80-55de-11eb-88da-44059899f9a9.png)
After this pr:
![image](https://user-images.githubusercontent.com/5399861/104452899-9a043600-55de-11eb-9286-d8f3a23ca3b8.png)
Closes#31145 from wangyum/SPARK-34081.
Authored-by: Yuming Wang <yumwang@ebay.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
XinDongSh pushed a commit to XinDongSh/spark that referenced this pull request Jan 20, 2021
wangyum added a commit that referenced this pull request May 26, 2023
…Anti over Aggregate if join can be planned as broadcast join
### What changes were proposed in this pull request?
Should not pushdown LeftSemi/LeftAnti over Aggregate for some cases.
```scala
spark.range(50000000L).selectExpr("id % 10000 as a", "id % 10000 as b").write.saveAsTable("t1")
spark.range(40000000L).selectExpr("id % 8000 as c", "id % 8000 as d").write.saveAsTable("t2")
spark.sql("SELECT distinct a, b FROM t1 INTERSECT SELECT distinct c, d FROM t2").explain
```
Before this pr:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#72]
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- SortMergeJoin [coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L)], [coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L)], LeftSemi
:- Sort [coalesce(a#16L, 0) ASC NULLS FIRST, isnull(a#16L) ASC NULLS FIRST, coalesce(b#17L, 0) ASC NULLS FIRST, isnull(b#17L) ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L), 5), ENSURE_REQUIREMENTS, [id=#65]
: +- FileScan parquet default.t1[a#16L,b#17L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:bigint,b:bigint>
+- Sort [coalesce(c#18L, 0) ASC NULLS FIRST, isnull(c#18L) ASC NULLS FIRST, coalesce(d#19L, 0) ASC NULLS FIRST, isnull(d#19L) ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L), 5), ENSURE_REQUIREMENTS, [id=#66]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- Exchange hashpartitioning(c#18L, d#19L, 5), ENSURE_REQUIREMENTS, [id=#61]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- FileScan parquet default.t2[c#18L,d#19L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<c:bigint,d:bigint>
```
After this pr:
```
== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#74]
+- HashAggregate(keys=[a#16L, b#17L], functions=[])
+- SortMergeJoin [coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L)], [coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L)], LeftSemi
:- Sort [coalesce(a#16L, 0) ASC NULLS FIRST, isnull(a#16L) ASC NULLS FIRST, coalesce(b#17L, 0) ASC NULLS FIRST, isnull(b#17L) ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(coalesce(a#16L, 0), isnull(a#16L), coalesce(b#17L, 0), isnull(b#17L), 5), ENSURE_REQUIREMENTS, [id=#67]
: +- HashAggregate(keys=[a#16L, b#17L], functions=[])
: +- Exchange hashpartitioning(a#16L, b#17L, 5), ENSURE_REQUIREMENTS, [id=#61]
: +- HashAggregate(keys=[a#16L, b#17L], functions=[])
: +- FileScan parquet default.t1[a#16L,b#17L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<a:bigint,b:bigint>
+- Sort [coalesce(c#18L, 0) ASC NULLS FIRST, isnull(c#18L) ASC NULLS FIRST, coalesce(d#19L, 0) ASC NULLS FIRST, isnull(d#19L) ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(coalesce(c#18L, 0), isnull(c#18L), coalesce(d#19L, 0), isnull(d#19L), 5), ENSURE_REQUIREMENTS, [id=#68]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- Exchange hashpartitioning(c#18L, d#19L, 5), ENSURE_REQUIREMENTS, [id=#63]
+- HashAggregate(keys=[c#18L, d#19L], functions=[])
+- FileScan parquet default.t2[c#18L,d#19L] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex[file:/Users/yumwang/spark/spark-warehouse/org.apache.spark.sql.Data..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<c:bigint,d:bigint>
```
### Why are the changes needed?
1. Pushdown LeftSemi/LeftAnti over Aggregate will affect performance.
2. It will remove user added DISTINCT operator, e.g.: [q38](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q38.sql), [q87](https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q87.sql).
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test and benchmark test.
SQL | Before this PR(Seconds) | After this PR(Seconds)
-- | -- | --
q14a | 660 | 594
q14b | 660 | 600
q38 | 55 | 29
q87 | 66 | 35
Before this pr:
![image](https://user-images.githubusercontent.com/5399861/104452849-8789fc80-55de-11eb-88da-44059899f9a9.png)
After this pr:
![image](https://user-images.githubusercontent.com/5399861/104452899-9a043600-55de-11eb-9286-d8f3a23ca3b8.png)
Closes#31145 from wangyum/SPARK-34081.
Authored-by: Yuming Wang <yumwang@ebay.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit d3ea308)
panbingkun pushed a commit that referenced this pull request Nov 22, 2024
…ead pool
### What changes were proposed in this pull request?
This PR aims to use a meaningful class name prefix for REST Submission API thread pool instead of the default value of Jetty QueuedThreadPool, `"qtp"+super.hashCode()`.
https://github.com/dekellum/jetty/blob/3dc0120d573816de7d6a83e2d6a97035288bdd4a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java#L64
### Why are the changes needed?
This is helpful during JVM investigation.
**BEFORE (4.0.0-preview2)**
```
$ SPARK_MASTER_OPTS='-Dspark.master.rest.enabled=true' sbin/start-master.sh
$ jstack 28217 | grep qtp
"qtp1925630411-52" #52 daemon prio=5 os_prio=31 cpu=0.07ms elapsed=19.06s tid=0x0000000134906c10 nid=0xde03 runnable [0x0000000314592000]
"qtp1925630411-53" #53 daemon prio=5 os_prio=31 cpu=0.05ms elapsed=19.06s tid=0x0000000134ac6810 nid=0xc603 runnable [0x000000031479e000]
"qtp1925630411-54" #54 daemon prio=5 os_prio=31 cpu=0.06ms elapsed=19.06s tid=0x000000013491ae10 nid=0xdc03 runnable [0x00000003149aa000]
"qtp1925630411-55" #55 daemon prio=5 os_prio=31 cpu=0.08ms elapsed=19.06s tid=0x0000000134ac9810 nid=0xc803 runnable [0x0000000314bb6000]
"qtp1925630411-56" #56 daemon prio=5 os_prio=31 cpu=0.04ms elapsed=19.06s tid=0x0000000134ac9e10 nid=0xda03 runnable [0x0000000314dc2000]
"qtp1925630411-57" #57 daemon prio=5 os_prio=31 cpu=0.05ms elapsed=19.06s tid=0x0000000134aca410 nid=0xca03 runnable [0x0000000314fce000]
"qtp1925630411-58" #58 daemon prio=5 os_prio=31 cpu=0.04ms elapsed=19.06s tid=0x0000000134acaa10 nid=0xcb03 runnable [0x00000003151da000]
"qtp1925630411-59" #59 daemon prio=5 os_prio=31 cpu=0.06ms elapsed=19.06s tid=0x0000000134acb010 nid=0xcc03 runnable [0x00000003153e6000]
"qtp1925630411-60-acceptor-0108e9815-ServerConnector1e497474{HTTP/1.1, (http/1.1)}{M3-Max.local:6066}" #60 daemon prio=3 os_prio=31 cpu=0.11ms elapsed=19.06s tid=0x00000001317ffa10 nid=0xcd03 runnable [0x00000003155f2000]
"qtp1925630411-61-acceptor-11d90f2aa-ServerConnector1e497474{HTTP/1.1, (http/1.1)}{M3-Max.local:6066}" #61 daemon prio=3 os_prio=31 cpu=0.10ms elapsed=19.06s tid=0x00000001314ed610 nid=0xcf03 waiting on condition [0x00000003157fe000]
```
**AFTER**
```
$ SPARK_MASTER_OPTS='-Dspark.master.rest.enabled=true' sbin/start-master.sh
$ jstack 28317 | grep StandaloneRestServer
"StandaloneRestServer-52" #52 daemon prio=5 os_prio=31 cpu=0.09ms elapsed=60.06s tid=0x00000001284a8e10 nid=0xdb03 runnable [0x000000032cfce000]
"StandaloneRestServer-53" #53 daemon prio=5 os_prio=31 cpu=0.06ms elapsed=60.06s tid=0x00000001284acc10 nid=0xda03 runnable [0x000000032d1da000]
"StandaloneRestServer-54" #54 daemon prio=5 os_prio=31 cpu=0.05ms elapsed=60.06s tid=0x00000001284ae610 nid=0xd803 runnable [0x000000032d3e6000]
"StandaloneRestServer-55" #55 daemon prio=5 os_prio=31 cpu=0.09ms elapsed=60.06s tid=0x00000001284aec10 nid=0xd703 runnable [0x000000032d5f2000]
"StandaloneRestServer-56" #56 daemon prio=5 os_prio=31 cpu=0.06ms elapsed=60.06s tid=0x00000001284af210 nid=0xc803 runnable [0x000000032d7fe000]
"StandaloneRestServer-57" #57 daemon prio=5 os_prio=31 cpu=0.05ms elapsed=60.06s tid=0x00000001284af810 nid=0xc903 runnable [0x000000032da0a000]
"StandaloneRestServer-58" #58 daemon prio=5 os_prio=31 cpu=0.06ms elapsed=60.06s tid=0x00000001284afe10 nid=0xcb03 runnable [0x000000032dc16000]
"StandaloneRestServer-59" #59 daemon prio=5 os_prio=31 cpu=0.05ms elapsed=60.06s tid=0x00000001284b0410 nid=0xcc03 runnable [0x000000032de22000]
"StandaloneRestServer-60-acceptor-04aefbaa8-ServerConnector44284d85{HTTP/1.1, (http/1.1)}{M3-Max.local:6066}" #60 daemon prio=3 os_prio=31 cpu=0.13ms elapsed=60.05s tid=0x000000015cda1a10 nid=0xcd03 runnable [0x000000032e02e000]
"StandaloneRestServer-61-acceptor-148976251-ServerConnector44284d85{HTTP/1.1, (http/1.1)}{M3-Max.local:6066}" #61 daemon prio=3 os_prio=31 cpu=0.12ms elapsed=60.05s tid=0x000000015cd1c810 nid=0xce03 waiting on condition [0x000000032e23a000]
```
### Does this PR introduce _any_ user-facing change?
No, the thread names are accessed during the debugging.
### How was this patch tested?
Manual review.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes#48924 from dongjoon-hyun/SPARK-50385.
Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: panbingkun <panbingkun@apache.org>
dongjoon-hyun added a commit that referenced this pull request Jul 21, 2025
…ingBuilder`
### What changes were proposed in this pull request?
This PR aims to improve `toString` by `JEP-280` instead of `ToStringBuilder`. In addition, `Scalastyle` and `Checkstyle` rules are added to prevent a future regression.
### Why are the changes needed?
Since Java 9, `String Concatenation` has been handled better by default.
| ID | DESCRIPTION |
| - | - |
| JEP-280 | [Indify String Concatenation](https://openjdk.org/jeps/280) |
For example, this PR improves `OpenBlocks` like the following. Both Java source code and byte code are simplified a lot by utilizing JEP-280 properly.
**CODE CHANGE**
```java
- return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
- .append("appId", appId)
- .append("execId", execId)
- .append("blockIds", Arrays.toString(blockIds))
- .toString();
+ return "OpenBlocks[appId=" + appId + ",execId=" + execId + ",blockIds=" +
+ Arrays.toString(blockIds) + "]";
```
**BEFORE**
```
public java.lang.String toString();
Code:
0: new #39 // class org/apache/commons/lang3/builder/ToStringBuilder
3: dup
4: aload_0
5: getstatic #41 // Field org/apache/commons/lang3/builder/ToStringStyle.SHORT_PREFIX_STYLE:Lorg/apache/commons/lang3/builder/ToStringStyle;
8: invokespecial #47 // Method org/apache/commons/lang3/builder/ToStringBuilder."<init>":(Ljava/lang/Object;Lorg/apache/commons/lang3/builder/ToStringStyle;)V
11: ldc #50 // String appId
13: aload_0
14: getfield #7 // Field appId:Ljava/lang/String;
17: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder;
20: ldc #55 // String execId
22: aload_0
23: getfield #13 // Field execId:Ljava/lang/String;
26: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder;
29: ldc #56 // String blockIds
31: aload_0
32: getfield #16 // Field blockIds:[Ljava/lang/String;
35: invokestatic #57 // Method java/util/Arrays.toString:([Ljava/lang/Object;)Ljava/lang/String;
38: invokevirtual #51 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder;
41: invokevirtual #61 // Method org/apache/commons/lang3/builder/ToStringBuilder.toString:()Ljava/lang/String;
44: areturn
```
**AFTER**
```
public java.lang.String toString();
Code:
0: aload_0
1: getfield #7 // Field appId:Ljava/lang/String;
4: aload_0
5: getfield #13 // Field execId:Ljava/lang/String;
8: aload_0
9: getfield #16 // Field blockIds:[Ljava/lang/String;
12: invokestatic #39 // Method java/util/Arrays.toString:([Ljava/lang/Object;)Ljava/lang/String;
15: invokedynamic #43, 0 // InvokeDynamic #0:makeConcatWithConstants:(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
20: areturn
```
### Does this PR introduce _any_ user-facing change?
No. This is an `toString` implementation improvement.
### How was this patch tested?
Pass the CIs.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes#51572 from dongjoon-hyun/SPARK-52880.
Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 1, 2026
### What changes were proposed in this pull request?
Adds support for `next_day(date, <weekday>)` (with a literal weekday, e.g. `next_day(d, 'MO')`) to Varka's vectorized SQL engine, so this function now runs on the fast path instead of falling back to row-by-row evaluation.
This was also a deliberate experiment: it's the first Varka task written as a step-by-step recipe and handed to an AI coding agent to execute on its own, to see how well that handover works. The recipe held up well overall.
A follow-up code review caught a real bug: under a rare combination of circumstances, a bad input could crash query planning instead of safely falling back to the normal (slower) execution path. That's fixed, along with several smaller documentation and test-coverage gaps the review found.
### Why are the changes needed?
`next_day` isn't a function real workloads use heavily, but it was cheap to add and useful as a trial run for delegating well-scoped tasks to an AI agent via a written recipe - which worked, with lessons recorded for next time.
### Does this PR introduce any user-facing change?
Yes. `next_day(date, <literal weekday>)` now runs on Varka's accelerated path. Queries using a non-literal weekday are unaffected (they already ran on the normal path and still do).
### How was this patch tested?
Full engine test suite passes on both supported CPU vector widths, including new tests for the bug the review found and edge cases (invalid weekday values, null handling, weekday boundaries).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Sonnet 5)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 1, 2026
Task 33 (apache#61) landed and both branches touch the compiler and the emitter.
The import conflicts in VarkaExpressionCompiler and its suite are the union of
the two sides: master's NextDay/DateTimeUtils/UTF8String additions plus this
branch's IntegerType and the byte/short type imports its decline tests need.
One conflict git did not report and would have merged into a broken build: this
branch renamed requireLiteralOffset to requireOffsetShape when it widened day
offsets to accept a column, and master added a *new* call to the old name for
next_day. Both checks are needed and they are not the same check. AddDays and
SubDays add their offset to a lane at run time, so a column is fine; next_day
folds its weekday into emit-time constants, so a non-literal there is not merely
unsupported but unrepresentable. requireLiteralOffset is restored alongside
requireOffsetShape, with a javadoc saying which is which and why, so the
guarantee is not quietly lost to the merge.
Green at both vector widths: catalyst 102, sql 132, and again under
-XX:MaxVectorSize=16; dev/lint-java and dev/scalastyle clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3VxmqDKhWHkvQ4Jrp6coG
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 1, 2026
Task 33 (apache#61) landed. Three conflicts, all additive.
The two import lines are the union of both sides: master's NextDay and
SparkIllegalArgumentException alongside this branch's UnixDate and
DateFromUnixDate, and in the suite master's Divide/EvalMode/NumericEvalContext
alongside this branch's relabelling expressions.
docs/sql-varka.md is a list both branches appended a bullet to; both bullets
stay, next_day first since it is the shipped feature and this branch's entry
still records a decline.
Green at both vector widths: catalyst 100, sql 129, and again under
-XX:MaxVectorSize=16; dev/lint-java and dev/scalastyle clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3VxmqDKhWHkvQ4Jrp6coG
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 1, 2026
Task 33 (apache#61) landed and both branches extend the emitter, the compiler and the
two pinned oracles.
Both weight constants survive: DAY_OF_YEAR_WEIGHT and NEXT_DAY_WEIGHT are
different nodes, and weightOf keeps this branch's precedence of testing
DayOfYear before isChrono, then master's NextDay arm, then the default. The
javadoc opener above the first constant had to be restored by hand - the
conflict markers sat inside a doc comment, so taking both sides left the second
block orphaned and javac rejected it, which is the sort of thing a merge tool
will hand you looking plausible.
The pinned oracles genuinely needed re-pinning rather than picking a side, since
each branch added a node to the same everyNode fixture and the union contains
both. `chrono` merged cleanly with DayOfYear, so the fixture takes master's
everyNode shape, which nests NextDay, and the resulting values were read off the
failing assertions rather than guessed: the shape hash is now 6b1350154da77f7e,
and the shallow rendering runs to 29=(if 10 13 28) with nextDay at 26 and
dayOfYear at 21. Both comments name both tasks.
Green at both vector widths: catalyst 100, sql 128, and again under
-XX:MaxVectorSize=16; dev/lint-java and dev/scalastyle clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3VxmqDKhWHkvQ4Jrp6coG
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 1, 2026
Task 33 (apache#61) landed. Both branches add an IR node, a compiler helper and an
entry to the two pinned oracles, so most of this is union work.
VarkaVectorIR's permits clause takes both NextDay and AddMonths. In
VarkaLoopEmitter both constants survive - ADD_MONTHS_TMP_COUNT and
NEXT_DAY_WEIGHT measure different things - and the javadoc opener above the
first had to be restored by hand, since the conflict markers sat inside a doc
comment and taking both sides otherwise leaves the second block orphaned.
weightOf merged cleanly: AddMonths is chrono and reuses CHRONO_WEIGHT by
design, so master's NextDay arm needed no adjustment.
In the compiler the second conflict is not a conflict at all but two unrelated
helpers, foldMonths and foldWeekday, that happened to land adjacent; both are
kept with their own doc comments. The import unions dropped a redundant
standalone VarkaVectorIR import that the merge would otherwise have left
alongside the braced one, which Scala rejects as unused.
The oracles are re-pinned rather than resolved to a side, because both branches
put their new node in the same fixture slot and the union has to nest them:
Least(WeekDay, Least(NextDay, AddMonths)). Values read off the failing
assertions, not guessed - shape hash 9e0f5388c7001183, and the rendering now
runs to 29=(if 10 13 28) with nextDay at 24 and addMonths at 25.
Green at both vector widths: catalyst 101, sql 128, and again under
-XX:MaxVectorSize=16; dev/lint-java and dev/scalastyle clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3VxmqDKhWHkvQ4Jrp6coG
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 2, 2026
### What changes were proposed in this pull request?
Two rows of `PLAN_MILESTONE_4.md`'s status table: task 33 (`next_day`, merged in apache#61) and task 40 (`add_months` / days-from-civil, merged in apache#67) gain the **DONE** marker, plan file and PR number that rows 24 and 26 already carry.
### Why are the changes needed?
Both merged without their table rows being updated, so a status read from the table alone understated the milestone by two tasks. Found while building a task-by-task status board from the table, the plan files and the merged-PR list.
### Does this PR introduce _any_ user-facing change?
No. Documentation only.
### How was this patch tested?
Not applicable - a two-line table edit; ASCII checked.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kayousterhout@mridulm@AmplabJenkins@shivaram@rxin