Skip to content

SPARK-1173. Improve scala streaming docs. - #64

Closed
kimballa wants to merge 2 commits into
apache:masterfrom
kimballa:spark-1173-streaming-docs
Closed

SPARK-1173. Improve scala streaming docs.#64
kimballa wants to merge 2 commits into
apache:masterfrom
kimballa:spark-1173-streaming-docs

Conversation

@kimballa

Copy link
Copy Markdown

Clarify imports to add implicit conversions to DStream and
fix other small typos in the streaming intro documentation.

Tested by inspecting output via a local jekyll server, c&p'ing the scala commands into a spark terminal.

Clarify imports to add implicit conversions to DStream and
fix other small typos in the streaming intro documentation.
@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@rxin

rxin commented Mar 3, 2014

Copy link
Copy Markdown
Contributor

Thanks Aaron. I've merged this.

@rxin

rxin commented Mar 3, 2014

Copy link
Copy Markdown
Contributor

There's also a typo in the Java version of the doc. If you don't mind fixing that as well ... :)

@kimballa

Copy link
Copy Markdown
Author

Here you go

@rxin

rxin commented Mar 3, 2014

Copy link
Copy Markdown
Contributor

Actually you will need to submit another PR. I've already merged this one (but github is laggy because it is waiting for the asf git bot to synchronize). Sorry about the confusion!

@kimballa

Copy link
Copy Markdown
Author

n.p. rebased and pushed to a new branch; see pull req #65.

@asfgitasfgit closed this in 2b53447Mar 3, 2014
asfgit pushed a commit that referenced this pull request Mar 3, 2014
Companion commit to pull request #64, fix the typo on the Java side of the docs.
Author: Aaron Kimball <aaron@magnify.io>
Closes#65 from kimballa/spark-1173-java-doc-update and squashes the following commits:
8ce11d3 [Aaron Kimball] SPARK-1173. (#2) Fix typo in Java streaming example.
tdas pushed a commit to tdas/spark that referenced this pull request Mar 20, 2014
Clarify imports to add implicit conversions to DStream and
fix other small typos in the streaming intro documentation.
Tested by inspecting output via a local jekyll server, c&p'ing the scala commands into a spark terminal.
Author: Aaron Kimball <aaron@magnify.io>
Closesapache#64 from kimballa/spark-1173-streaming-docs and squashes the following commits:
6fbff0e [Aaron Kimball] SPARK-1173. Improve scala streaming docs.
tdas pushed a commit to tdas/spark that referenced this pull request Mar 20, 2014
Companion commit to pull request apache#64, fix the typo on the Java side of the docs.
Author: Aaron Kimball <aaron@magnify.io>
Closesapache#65 from kimballa/spark-1173-java-doc-update and squashes the following commits:
8ce11d3 [Aaron Kimball] SPARK-1173. (#2) Fix typo in Java streaming example.
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
robert3005 added a commit to robert3005/spark that referenced this pull request Jan 12, 2017
ghost pushed a commit to dbtsai/spark that referenced this pull request Feb 16, 2017
## What changes were proposed in this pull request?
These error below seems caused by unidoc that does not understand double commented block.
```
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:69: error: class, interface, or enum expected
[error] * MapGroupsWithStateFunction&lt;String, Integer, Integer, String&gt; mappingFunction =
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:69: error: class, interface, or enum expected
[error] * MapGroupsWithStateFunction&lt;String, Integer, Integer, String&gt; mappingFunction =
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:70: error: class, interface, or enum expected
[error] * new MapGroupsWithStateFunction&lt;String, Integer, Integer, String&gt;() {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:70: error: class, interface, or enum expected
[error] * new MapGroupsWithStateFunction&lt;String, Integer, Integer, String&gt;() {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:72: error: illegal character: '#'
[error] * &apache#64;Override
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:72: error: class, interface, or enum expected
[error] * &apache#64;Override
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:73: error: class, interface, or enum expected
[error] * public String call(String key, Iterator&lt;Integer&gt; value, KeyedState&lt;Integer&gt; state) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:73: error: class, interface, or enum expected
[error] * public String call(String key, Iterator&lt;Integer&gt; value, KeyedState&lt;Integer&gt; state) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:73: error: class, interface, or enum expected
[error] * public String call(String key, Iterator&lt;Integer&gt; value, KeyedState&lt;Integer&gt; state) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:73: error: class, interface, or enum expected
[error] * public String call(String key, Iterator&lt;Integer&gt; value, KeyedState&lt;Integer&gt; state) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:73: error: class, interface, or enum expected
[error] * public String call(String key, Iterator&lt;Integer&gt; value, KeyedState&lt;Integer&gt; state) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:76: error: class, interface, or enum expected
[error] * boolean shouldRemove = ...; // Decide whether to remove the state
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:77: error: class, interface, or enum expected
[error] * if (shouldRemove) {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:79: error: class, interface, or enum expected
[error] * } else {
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:81: error: class, interface, or enum expected
[error] * state.update(newState); // Set the new state
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:82: error: class, interface, or enum expected
[error] * }
[error] ^
[error] .../forked/spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:85: error: class, interface, or enum expected
[error] * state.update(initialState);
[error] ^
[error] .../forked/spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:86: error: class, interface, or enum expected
[error] * }
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:90: error: class, interface, or enum expected
[error] * </code></pre>
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:92: error: class, interface, or enum expected
[error] * tparam S User-defined type of the state to be stored for each key. Must be encodable into
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:93: error: class, interface, or enum expected
[error] * Spark SQL types (see {link Encoder} for more details).
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:94: error: class, interface, or enum expected
[error] * since 2.1.1
[error] ^
```
And another link seems unrecognisable.
```
.../spark/sql/core/target/java/org/apache/spark/sql/KeyedState.java:16: error: reference not found
[error] * That is, in every batch of the {link streaming.StreamingQuery StreamingQuery},
[error]
```
Note that this PR does not fix the two breaks as below:
```
[error] .../spark/sql/core/target/java/org/apache/spark/sql/DataFrameStatFunctions.java:43: error: unexpected content
[error] * see {link DataFrameStatsFunctions.approxQuantile(col:Str* approxQuantile} for
[error] ^
[error] .../spark/sql/core/target/java/org/apache/spark/sql/DataFrameStatFunctions.java:52: error: bad use of '>'
[error] * param relativeError The relative target precision to achieve (>= 0).
[error] ^
[error]
```
because these seem probably fixed soon in apache#16776 and I intended to avoid potential conflicts.
## How was this patch tested?
Manually via `jekyll build`
Author: hyukjinkwon <gurwls223@gmail.com>
Closesapache#16926 from HyukjinKwon/javadoc-break.
jlopezmalla added a commit to jlopezmalla/spark that referenced this pull request Sep 19, 2017
* fix History Server entrypoint permision bug
* remove skipped test
remove skipped test
Igosuki pushed a commit to Adikteev/spark that referenced this pull request Jul 31, 2018
[SPARK-344] Revert "Implement a Go Spark CLI"
su8su pushed a commit to su8su/spark that referenced this pull request Dec 11, 2018
…on number is not same (apache#64)
* disable auto calculate the reduce number when the pre-shuffle partition number is not same
* add the hashpartition check for ae
* style
weixiuli pushed a commit to weixiuli/spark that referenced this pull request Jun 18, 2019
…on number is not same (apache#64)
* disable auto calculate the reduce number when the pre-shuffle partition number is not same
* add the hashpartition check for ae
* style
hejian991 pushed a commit to growingio/spark that referenced this pull request Jun 24, 2019
…on number is not same (apache#64)
* disable auto calculate the reduce number when the pre-shuffle partition number is not same
* add the hashpartition check for ae
* style
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
* Fix the opentelekomcloud job
- Add the OS_VPC_ID environment variable
- Change the OS_FLAVOR_ID and OS_FLAVOR_ID_RESIZE value
- Enable all the tests running
For apache#63
* Update run.yaml
* change to use an ubuntu image for testing
* add az environment variable for otc job
hn5092 pushed a commit to hn5092/spark that referenced this pull request Nov 4, 2019
apache#62 [SPARK-29284][SQL] Adaptive query execution works correctly when …
maropu pushed a commit that referenced this pull request Jan 23, 2020
… and aggregates
### What changes were proposed in this pull request?
Currently, in the following scenario, bucket join is not utilized:
```scala
val df = (0 until 20).map(i => (i, i)).toDF("i", "j").as("df")
df.write.format("parquet").bucketBy(8, "i").saveAsTable("t")
sql("CREATE VIEW v AS SELECT * FROM t")
sql("SELECT * FROM t a JOIN v b ON a.i = b.i").explain
```
```
== Physical Plan ==
*(4) SortMergeJoin [i#13], [i#15], Inner
:- *(1) Sort [i#13 ASC NULLS FIRST], false, 0
: +- *(1) Project [i#13, j#14]
: +- *(1) Filter isnotnull(i#13)
: +- *(1) ColumnarToRow
: +- FileScan parquet default.t[i#13,j#14] Batched: true, DataFilters: [isnotnull(i#13)], Format: Parquet, Location: InMemoryFileIndex[file:..., PartitionFilters: [], PushedFilters: [IsNotNull(i)], ReadSchema: struct<i:int,j:int>, SelectedBucketsCount: 8 out of 8
+- *(3) Sort [i#15 ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(i#15, 8), true, [id=#64] <----- Exchange node introduced
+- *(2) Project [i#13 AS i#15, j#14 AS j#16]
+- *(2) Filter isnotnull(i#13)
+- *(2) ColumnarToRow
+- FileScan parquet default.t[i#13,j#14] Batched: true, DataFilters: [isnotnull(i#13)], Format: Parquet, Location: InMemoryFileIndex[file:..., PartitionFilters: [], PushedFilters: [IsNotNull(i)], ReadSchema: struct<i:int,j:int>, SelectedBucketsCount: 8 out of 8
```
Notice that `Exchange` is present. This is because `Project` introduces aliases and `outputPartitioning` and `requiredChildDistribution` do not consider aliases while considering bucket join in `EnsureRequirements`. This PR addresses to allow this scenario.
### Why are the changes needed?
This allows bucket join to be utilized in the above example.
### Does this PR introduce any user-facing change?
Yes, now with the fix, the `explain` out is as follows:
```
== Physical Plan ==
*(3) SortMergeJoin [i#13], [i#15], Inner
:- *(1) Sort [i#13 ASC NULLS FIRST], false, 0
: +- *(1) Project [i#13, j#14]
: +- *(1) Filter isnotnull(i#13)
: +- *(1) ColumnarToRow
: +- FileScan parquet default.t[i#13,j#14] Batched: true, DataFilters: [isnotnull(i#13)], Format: Parquet, Location: InMemoryFileIndex[file:.., PartitionFilters: [], PushedFilters: [IsNotNull(i)], ReadSchema: struct<i:int,j:int>, SelectedBucketsCount: 8 out of 8
+- *(2) Sort [i#15 ASC NULLS FIRST], false, 0
+- *(2) Project [i#13 AS i#15, j#14 AS j#16]
+- *(2) Filter isnotnull(i#13)
+- *(2) ColumnarToRow
+- FileScan parquet default.t[i#13,j#14] Batched: true, DataFilters: [isnotnull(i#13)], Format: Parquet, Location: InMemoryFileIndex[file:.., PartitionFilters: [], PushedFilters: [IsNotNull(i)], ReadSchema: struct<i:int,j:int>, SelectedBucketsCount: 8 out of 8
```
Note that the `Exchange` is no longer present.
### How was this patch tested?
Closes#26943 from imback82/bucket_alias.
Authored-by: Terry Kim <yuminkim@gmail.com>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
hvanhovell pushed a commit that referenced this pull request Feb 13, 2020
…in optimizations
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This is a followup of #26434
This PR use one special shuffle reader for skew join, so that we only have one join after optimization. In order to do that, this PR
1. add a very general `CustomShuffledRowRDD` which support all kind of partition arrangement.
2. move the logic of coalescing shuffle partitions to a util function, and call it during skew join optimization, to totally decouple with the `ReduceNumShufflePartitions` rule. It's too complicated to interfere skew join with `ReduceNumShufflePartitions`, as you need to consider the size of split partitions which don't respect target size already.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The current skew join optimization has a serious performance issue: the size of the query plan depends on the number and size of skewed partitions.
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
no
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
existing tests
test UI manually:
![image](https://user-images.githubusercontent.com/3182036/74357390-cfb30480-4dfa-11ea-83f6-825d1b9379ca.png)
explain output
```
AdaptiveSparkPlan(isFinalPlan=true)
+- OverwriteByExpression org.apache.spark.sql.execution.datasources.noop.NoopTable$403a2ed5, [AlwaysTrue()], org.apache.spark.sql.util.CaseInsensitiveStringMap1f
+- *(5) SortMergeJoin(skew=true) [key1#2L], [key2#6L], Inner
:- *(3) Sort [key1#2L ASC NULLS FIRST], false, 0
: +- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
: +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key1#2L, 200), true, [id=#53]
: +- *(1) Project [(id#0L % 2) AS key1#2L]
: +- *(1) Filter isnotnull((id#0L % 2))
: +- *(1) Range (0, 100000, step=1, splits=6)
+- *(4) Sort [key2#6L ASC NULLS FIRST], false, 0
+- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
+- ShuffleQueryStage 1
+- Exchange hashpartitioning(key2#6L, 200), true, [id=#64]
+- *(2) Project [((id#4L % 2) + 1) AS key2#6L]
+- *(2) Filter isnotnull(((id#4L % 2) + 1))
+- *(2) Range (0, 100000, step=1, splits=6)
```
Closes#27493 from cloud-fan/aqe.
Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: herman <herman@databricks.com>
hvanhovell pushed a commit that referenced this pull request Feb 13, 2020
…in optimizations
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This is a followup of #26434
This PR use one special shuffle reader for skew join, so that we only have one join after optimization. In order to do that, this PR
1. add a very general `CustomShuffledRowRDD` which support all kind of partition arrangement.
2. move the logic of coalescing shuffle partitions to a util function, and call it during skew join optimization, to totally decouple with the `ReduceNumShufflePartitions` rule. It's too complicated to interfere skew join with `ReduceNumShufflePartitions`, as you need to consider the size of split partitions which don't respect target size already.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The current skew join optimization has a serious performance issue: the size of the query plan depends on the number and size of skewed partitions.
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
no
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
existing tests
test UI manually:
![image](https://user-images.githubusercontent.com/3182036/74357390-cfb30480-4dfa-11ea-83f6-825d1b9379ca.png)
explain output
```
AdaptiveSparkPlan(isFinalPlan=true)
+- OverwriteByExpression org.apache.spark.sql.execution.datasources.noop.NoopTable$403a2ed5, [AlwaysTrue()], org.apache.spark.sql.util.CaseInsensitiveStringMap1f
+- *(5) SortMergeJoin(skew=true) [key1#2L], [key2#6L], Inner
:- *(3) Sort [key1#2L ASC NULLS FIRST], false, 0
: +- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
: +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key1#2L, 200), true, [id=#53]
: +- *(1) Project [(id#0L % 2) AS key1#2L]
: +- *(1) Filter isnotnull((id#0L % 2))
: +- *(1) Range (0, 100000, step=1, splits=6)
+- *(4) Sort [key2#6L ASC NULLS FIRST], false, 0
+- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
+- ShuffleQueryStage 1
+- Exchange hashpartitioning(key2#6L, 200), true, [id=#64]
+- *(2) Project [((id#4L % 2) + 1) AS key2#6L]
+- *(2) Filter isnotnull(((id#4L % 2) + 1))
+- *(2) Range (0, 100000, step=1, splits=6)
```
Closes#27493 from cloud-fan/aqe.
Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: herman <herman@databricks.com>
(cherry picked from commit a4ceea6)
Signed-off-by: herman <herman@databricks.com>
sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
…in optimizations
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This is a followup of apache#26434
This PR use one special shuffle reader for skew join, so that we only have one join after optimization. In order to do that, this PR
1. add a very general `CustomShuffledRowRDD` which support all kind of partition arrangement.
2. move the logic of coalescing shuffle partitions to a util function, and call it during skew join optimization, to totally decouple with the `ReduceNumShufflePartitions` rule. It's too complicated to interfere skew join with `ReduceNumShufflePartitions`, as you need to consider the size of split partitions which don't respect target size already.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The current skew join optimization has a serious performance issue: the size of the query plan depends on the number and size of skewed partitions.
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
no
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
existing tests
test UI manually:
![image](https://user-images.githubusercontent.com/3182036/74357390-cfb30480-4dfa-11ea-83f6-825d1b9379ca.png)
explain output
```
AdaptiveSparkPlan(isFinalPlan=true)
+- OverwriteByExpression org.apache.spark.sql.execution.datasources.noop.NoopTable$403a2ed5, [AlwaysTrue()], org.apache.spark.sql.util.CaseInsensitiveStringMap1f
+- *(5) SortMergeJoin(skew=true) [key1#2L], [key2#6L], Inner
:- *(3) Sort [key1#2L ASC NULLS FIRST], false, 0
: +- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
: +- ShuffleQueryStage 0
: +- Exchange hashpartitioning(key1#2L, 200), true, [id=apache#53]
: +- *(1) Project [(id#0L % 2) AS key1#2L]
: +- *(1) Filter isnotnull((id#0L % 2))
: +- *(1) Range (0, 100000, step=1, splits=6)
+- *(4) Sort [key2#6L ASC NULLS FIRST], false, 0
+- SkewJoinShuffleReader 2 skewed partitions with size(max=5 KB, min=5 KB, avg=5 KB)
+- ShuffleQueryStage 1
+- Exchange hashpartitioning(key2#6L, 200), true, [id=apache#64]
+- *(2) Project [((id#4L % 2) + 1) AS key2#6L]
+- *(2) Filter isnotnull(((id#4L % 2) + 1))
+- *(2) Range (0, 100000, step=1, splits=6)
```
Closesapache#27493 from cloud-fan/aqe.
Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: herman <herman@databricks.com>
cloud-fan pushed a commit that referenced this pull request Oct 8, 2020
… more scenarios such as PartitioningCollection
### What changes were proposed in this pull request?
This PR proposes to improve `EnsureRquirement.reorderJoinKeys` to handle the following scenarios:
1. If the keys cannot be reordered to match the left-side `HashPartitioning`, consider the right-side `HashPartitioning`.
2. Handle `PartitioningCollection`, which may contain `HashPartitioning`
### Why are the changes needed?
1. For the scenario 1), the current behavior matches either the left-side `HashPartitioning` or the right-side `HashPartitioning`. This means that if both sides are `HashPartitioning`, it will try to match only the left side.
The following will not consider the right-side `HashPartitioning`:
```
val df1 = (0 until 10).map(i => (i % 5, i % 13)).toDF("i1", "j1")
val df2 = (0 until 10).map(i => (i % 7, i % 11)).toDF("i2", "j2")
df1.write.format("parquet").bucketBy(4, "i1", "j1").saveAsTable("t1")df2.write.format("parquet").bucketBy(4, "i2", "j2").saveAsTable("t2")
val t1 = spark.table("t1")
val t2 = spark.table("t2")
val join = t1.join(t2, t1("i1") === t2("j2") && t1("i1") === t2("i2"))
join.explain
== Physical Plan ==
*(5) SortMergeJoin [i1#26, i1#26], [j2#31, i2#30], Inner
:- *(2) Sort [i1#26 ASC NULLS FIRST, i1#26 ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(i1#26, i1#26, 4), true, [id=#69]
: +- *(1) Project [i1#26, j1#27]
: +- *(1) Filter isnotnull(i1#26)
: +- *(1) ColumnarToRow
: +- FileScan parquet default.t1[i1#26,j1#27] Batched: true, DataFilters: [isnotnull(i1#26)], Format: Parquet, Location: InMemoryFileIndex[..., PartitionFilters: [], PushedFilters: [IsNotNull(i1)], ReadSchema: struct<i1:int,j1:int>, SelectedBucketsCount: 4 out of 4
+- *(4) Sort [j2#31 ASC NULLS FIRST, i2#30 ASC NULLS FIRST], false, 0.
+- Exchange hashpartitioning(j2#31, i2#30, 4), true, [id=#79]. <===== This can be removed
+- *(3) Project [i2#30, j2#31]
+- *(3) Filter (((j2#31 = i2#30) AND isnotnull(j2#31)) AND isnotnull(i2#30))
+- *(3) ColumnarToRow
+- FileScan parquet default.t2[i2#30,j2#31] Batched: true, DataFilters: [(j2#31 = i2#30), isnotnull(j2#31), isnotnull(i2#30)], Format: Parquet, Location: InMemoryFileIndex[..., PartitionFilters: [], PushedFilters: [IsNotNull(j2), IsNotNull(i2)], ReadSchema: struct<i2:int,j2:int>, SelectedBucketsCount: 4 out of 4
```
2. For the scenario 2), the current behavior does not handle `PartitioningCollection`:
```
val df1 = (0 until 100).map(i => (i % 5, i % 13)).toDF("i1", "j1")
val df2 = (0 until 100).map(i => (i % 7, i % 11)).toDF("i2", "j2")
val df3 = (0 until 100).map(i => (i % 5, i % 13)).toDF("i3", "j3")
val join = df1.join(df2, df1("i1") === df2("i2") && df1("j1") === df2("j2")) // PartitioningCollection
val join2 = join.join(df3, join("j1") === df3("j3") && join("i1") === df3("i3"))
join2.explain
== Physical Plan ==
*(9) SortMergeJoin [j1#8, i1#7], [j3#30, i3#29], Inner
:- *(6) Sort [j1#8 ASC NULLS FIRST, i1#7 ASC NULLS FIRST], false, 0. <===== This can be removed
: +- Exchange hashpartitioning(j1#8, i1#7, 5), true, [id=#58] <===== This can be removed
: +- *(5) SortMergeJoin [i1#7, j1#8], [i2#18, j2#19], Inner
: :- *(2) Sort [i1#7 ASC NULLS FIRST, j1#8 ASC NULLS FIRST], false, 0
: : +- Exchange hashpartitioning(i1#7, j1#8, 5), true, [id=#45]
: : +- *(1) Project [_1#2 AS i1#7, _2#3 AS j1#8]
: : +- *(1) LocalTableScan [_1#2, _2#3]
: +- *(4) Sort [i2#18 ASC NULLS FIRST, j2#19 ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(i2#18, j2#19, 5), true, [id=#51]
: +- *(3) Project [_1#13 AS i2#18, _2#14 AS j2#19]
: +- *(3) LocalTableScan [_1#13, _2#14]
+- *(8) Sort [j3#30 ASC NULLS FIRST, i3#29 ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(j3#30, i3#29, 5), true, [id=#64]
+- *(7) Project [_1#24 AS i3#29, _2#25 AS j3#30]
+- *(7) LocalTableScan [_1#24, _2#25]
```
### Does this PR introduce _any_ user-facing change?
Yes, now from the above examples, the shuffle/sort nodes pointed by `This can be removed` are now removed:
1. Senario 1):
```
== Physical Plan ==
*(4) SortMergeJoin [i1#26, i1#26], [i2#30, j2#31], Inner
:- *(2) Sort [i1#26 ASC NULLS FIRST, i1#26 ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(i1#26, i1#26, 4), true, [id=#67]
: +- *(1) Project [i1#26, j1#27]
: +- *(1) Filter isnotnull(i1#26)
: +- *(1) ColumnarToRow
: +- FileScan parquet default.t1[i1#26,j1#27] Batched: true, DataFilters: [isnotnull(i1#26)], Format: Parquet, Location: InMemoryFileIndex[..., PartitionFilters: [], PushedFilters: [IsNotNull(i1)], ReadSchema: struct<i1:int,j1:int>, SelectedBucketsCount: 4 out of 4
+- *(3) Sort [i2#30 ASC NULLS FIRST, j2#31 ASC NULLS FIRST], false, 0
+- *(3) Project [i2#30, j2#31]
+- *(3) Filter (((j2#31 = i2#30) AND isnotnull(j2#31)) AND isnotnull(i2#30))
+- *(3) ColumnarToRow
+- FileScan parquet default.t2[i2#30,j2#31] Batched: true, DataFilters: [(j2#31 = i2#30), isnotnull(j2#31), isnotnull(i2#30)], Format: Parquet, Location: InMemoryFileIndex[..., PartitionFilters: [], PushedFilters: [IsNotNull(j2), IsNotNull(i2)], ReadSchema: struct<i2:int,j2:int>, SelectedBucketsCount: 4 out of 4
```
2. Scenario 2):
```
== Physical Plan ==
*(8) SortMergeJoin [i1#7, j1#8], [i3#29, j3#30], Inner
:- *(5) SortMergeJoin [i1#7, j1#8], [i2#18, j2#19], Inner
: :- *(2) Sort [i1#7 ASC NULLS FIRST, j1#8 ASC NULLS FIRST], false, 0
: : +- Exchange hashpartitioning(i1#7, j1#8, 5), true, [id=#43]
: : +- *(1) Project [_1#2 AS i1#7, _2#3 AS j1#8]
: : +- *(1) LocalTableScan [_1#2, _2#3]
: +- *(4) Sort [i2#18 ASC NULLS FIRST, j2#19 ASC NULLS FIRST], false, 0
: +- Exchange hashpartitioning(i2#18, j2#19, 5), true, [id=#49]
: +- *(3) Project [_1#13 AS i2#18, _2#14 AS j2#19]
: +- *(3) LocalTableScan [_1#13, _2#14]
+- *(7) Sort [i3#29 ASC NULLS FIRST, j3#30 ASC NULLS FIRST], false, 0
+- Exchange hashpartitioning(i3#29, j3#30, 5), true, [id=#58]
+- *(6) Project [_1#24 AS i3#29, _2#25 AS j3#30]
+- *(6) LocalTableScan [_1#24, _2#25]
```
### How was this patch tested?
Added tests.
Closes#29074 from imback82/reorder_keys.
Authored-by: Terry Kim <yuminkim@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
XinDongSh pushed a commit to XinDongSh/spark that referenced this pull request Feb 3, 2021
HyukjinKwon pushed a commit that referenced this pull request Feb 4, 2021
### What changes were proposed in this pull request?
Skip the zinc related installation operations on aarch64 platform.
### Why are the changes needed?
The standalone zinc is not supported well on aarch64, so that the error ouput, `cannot execute binary file: Exec format error` dumped after build/mvn is called.
This patch try to skip the zinc installation and related operations on aarch64 to make sure the error output doesn't print again on aarch64.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
simple cmd: `build/mvn -v`, see no error ouput again in aarch64, and nothing changed on x86
- on AArch64 Ubuntu
```
rootyikun-arm:~/dev/spark# uname -a
Linux yikun-arm 4.15.0-70-generic #79-Ubuntu SMP Tue Nov 12 10:36:10 UTC 2019 aarch64 aarch64 aarch64 GNU/Linux
rootyikun-arm:~/dev/spark# uname -m
aarch64
rootyikun-arm:~/dev/spark# build/mvn -v
Using `mvn` from path: /root/dev/spark/build/apache-maven-3.6.3/bin/mvn
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /root/dev/spark/build/apache-maven-3.6.3
Java version: 1.8.0_222, vendor: Private Build, runtime: /usr/lib/jvm/java-8-openjdk-arm64/jre
Default locale: en, platform encoding: UTF-8
OS name: "linux", version: "4.15.0-70-generic", arch: "aarch64", family: "unix"
```
- on x86 Mac OS
```
# uname -a
Darwin MacBook.local 19.6.0 Darwin Kernel Version 19.6.0: Tue Nov 10 00:10:30 PST 2020; root:xnu-6153.141.10~1/RELEASE_X86_64 x86_64
# uname -m
x86_64
# build/mvn -v
Using `mvn` from path: /Users/jiangyikun/huawei/apache-maven-3.6.3/bin/mvn
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /Users/jiangyikun/huawei/apache-maven-3.6.3
Java version: 1.8.0_221, vendor: Oracle Corporation, runtime: /Library/Java/JavaVirtualMachines/jdk1.8.0_221.jdk/Contents/Home/jre
Default locale: zh_CN, platform encoding: UTF-8
OS name: "mac os x", version: "10.15.7", arch: "x86_64", family: "mac"
```
- on x86 Ubuntu
```
rootyikun-x86:~/spark# uname -a
Linux yikun-x86 5.4.0-58-generic #64-Ubuntu SMP Wed Dec 9 08:16:25 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
rootyikun-x86:~/spark# uname -m
x86_64
rootyikun-x86:~/spark# ./build//mvn -v
Using `mvn` from path: /root/spark/build/apache-maven-3.6.3/bin/mvn
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /root/spark/build/apache-maven-3.6.3
Java version: 1.8.0_275, vendor: Private Build, runtime: /usr/lib/jvm/java-8-openjdk-amd64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.4.0-58-generic", arch: "amd64", family: "unix"
```
Closes#31454 from Yikun/zinc_skip_aarch64.
Authored-by: Yikun Jiang <yikunkero@gmail.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
VandanaSah pushed a commit to VandanaSah/spark that referenced this pull request Apr 13, 2026
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 2, 2026
… budget (apache#74)
### What changes were proposed in this pull request?
A plan only, no code: section 10 of `PLAN_TASK_32.md`, plus a pointer from `PLAN_MILESTONE_4.md`'s task-32 row and section 2.9.
Step B2 is the grouping change that turns step B1's fragment sharing into a hot-loop win. Today every calendar output still gets its own loop method (`CHRONO_WEIGHT` 50 against `GROUP_BUDGET` 16), so the shared civil-from-days prefix only ever fires in the epilogue. The gate measured B2's shape at 1.29x/1.57x/1.80x for two/three/four fields at AVX-512 and nearly the same at 128-bit (`PLAN_TASK_32.md` 7.2, 7.4), and measured the compile cliff away (7.5) - but it did so by forcing `groupBudget` to 200, which is the measurement rig, not a shippable rule: a wide budget also merges plain chains, and task 17 measured that losing (4436 vs 3150 M rows/s in the committed parity file).
The plan replaces that with a rule. `addOps` counts a fragment's prefix once per group and reports what an output saved by reusing one; `groupOutputs` gains a second join clause that admits an output past `GROUP_BUDGET` only when `saved > 0`, bounded by a new `fusedCeiling` emit option. That is section 3.2's original sketch with one correction: 3.2 wrote the test as `marginal <= budget`, which would have re-merged task 17's case (marginal 6); `saved > 0` keeps it split. Section 10.2 works the rule through the shapes this task has measured, including one recorded limitation (greedy output order can leave `month(d)` in a group without `year(d)`; correct, no worse than today, parked in the debt register).
The rest: weights re-counted from emitted instructions because they now bound a method (10.3); a ladder past four fields - 6, 8, 12 outputs over one date via `add_months(d, k)` literals - that sets `fusedCeiling` on throughput and `-XX:+PrintCompilation` time at both widths (10.4); which tests flip and which guard is added (10.5, including a byte-identity check over non-calendar shapes so clause 2 cannot leak past fragments); six registered predictions, one of them for task 45 to score (10.6); risks (10.7); a four-commit sequence after apache#73 and apache#64 have landed (10.8); and what stays out (10.9).
### Why are the changes needed?
The gate for B2 has cleared at both widths and the remaining work is a design choice with a measured wrong answer available (the wide budget). Writing the rule down before building it is the project's standing practice, and this one has a specific trap - 3.2's own sketch re-merges task 17's case - that is cheaper to catch on paper than in a parity regeneration diff.
### Does this PR introduce _any_ user-facing change?
No. Documentation only.
### How was this patch tested?
Not applicable - plan files only. ASCII checked; the prose width matches the file's existing convention.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Fable 5.1)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 2, 2026
### What changes were proposed in this pull request?
`emitLeapFlag` evaluated the Gregorian rule directly: two magic divisions, `% 100` and `% 400`, each a round-down quotient plus a correction carry, over a biased year. Nineteen int-vector ops and three mask ops, five scratch locals, and seven parameters — five of which existed only to thread those locals in from the caller.
Falk Hüffner's leap-year test does the same job as a perfect hash, and it maps onto a lane cleanly:
```java
y.add(YEAR_BIAS).mul(1073750999).and(0xC001F00F).compare(VectorOperators.ULE, 126976)
```
Four int-vector ops, no mask ops, no scratch, two parameters.
**Measured**, on `add_months` — the only caller on master — against the baseline apache#82 commits, minimum of three regenerations each:
| case | before | after | |
|---|---|---|---|
| `add_months(d, 13), null-free` | 563.9 | **642.4** M rows/s | 1.14x |
| `add_months(d, 13), mixed nulls` | 557.7 | **630.9** M rows/s | 1.13x |
| `per-row LocalDate year` (in-run control) | 481.8 | 481.4 | 1.00x |
The control is flat across both halves, so the machine did not move between them; each half's own spread is under 1%, and 1.14x is well outside the ±15% band this repo re-checks ratios against. Registered before measuring: a real latency win was expected but might land inside noise, the way task 48's four ops did. It did not — 22 lane ops off a node of about 120 is a large enough slice to show.
**Three things about the lowering are not obvious**, and each is documented where a reader will hit it, because all three look like bugs:
- **The multiply overflows the lane, deliberately.** The identity is defined modulo 2³², which is exactly what an `int` multiply computes, so the wrap is the mechanism and must not be "fixed".
- **The compare must be unsigned.** The mask keeps bits 30 and 31, so 51,250 of the 102,500 years in the domain leave a negative `int`; a signed compare would call every one of them leap. The Vector API spells it `ULE`.
- **A perfect hash is exact inside its domain and arbitrary one step outside**, so the domain is the whole contract, not a quality-of-approximation note. The covered reported years are −15200..87299, against the roughly −14848..35181 that `add_months` and the interval arithmetic can reach. A caller outside that needs a different bias, not a correction.
**This supersedes the planned `emitLeapFlag` consolidation.** Task 34 (apache#64) and task 40 each derived one of these helpers, differing only in bias, and the open question was which bias to keep. Both are replaced, and the seven-parameter list they disagreed about is gone. apache#64 and apache#78 both call this helper and will each drop five arguments at their call site when they merge — a compile error, not a silent misuse.
`VarkaChrono` gains `isLeapYear` as the scalar twin, computing it the same way, so a disagreement between model and kernel stays an emission bug rather than an arithmetic one. `add_months` keeps three of the five scratch locals because `emitDaysFromCivil` reuses them; they are renamed, since they are no longer about leap years, and `ADD_MONTHS_TMP_COUNT` drops from 33 to 31.
### Why are the changes needed?
`add_months` is the weakest calendar node in the parity file — apache#82 measures it at 1.17x the row engine's own path, where `year` is 4.5x — and 22 of its roughly 120 lane ops were spent deciding one bit.
### Does this PR introduce _any_ user-facing change?
No. The emitted arithmetic changes; the answers do not, which the exhaustive sweep is what establishes.
### How was this patch tested?
The constants were verified on this JVM rather than taken from the source article: zero mismatches against the Gregorian rule over every reported year the bias covers, −15200..87299, at 16 lanes and at 4. `VarkaChronoSuite` now runs that sweep over all 102,500 years as a committed test, and asserts **both** ends of the domain — including that year 87300 is still wrong — so that a future widening of the range trips there rather than in a differential. A second test holds `isLeapYear` to `java.time` over the suite's calendar-boundary set.
Every Varka suite green in catalyst and sql at both vector widths (`-XX:MaxVectorSize=16` through the forked test JVM's `Test / javaOptions`), with the opt-in exhaustive sweeps on.
No pinned fixture moves, which is worth stating because the emitted bytes for `add_months` change substantially: the shape hash and the line map pin the IR and the line mapping, not the arithmetic, and every op here carries the `AddMonths` node's own line.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 3, 2026
### What changes were proposed in this pull request?
In the PR, I propose a `year` that does not compute the month, in four commits (plan, identity, elision, measurement), plus the plan for a follow-on task the work turned up.
The year tail needs one bit out of the March-based month `mp`: whether the March year has turned January, which is `mp >= 10`. Since `mp = (5 * doy + 2) / 153` is exact over the whole domain the prefix leaves, that bit is `doy >= 306` — an integer identity, proved in three lines and then asserted over all 366 values of the domain through the shipped constants. So `emitChronoYear` reads the day of year, and the prefix's month step becomes dead work in any body whose calendar tails are all years.
`PLAN_MILESTONE_4.md` 2.18 predicted this task and task 32 step B were in tension: once the prefix is shared it computes `mp` for the month, day-of-month and quarter tails regardless, so "whichever of the two lands second inherits the smaller half of the win" — one op instead of five. Step B1 landed first. **This does not take the smaller half.** The step is dead work exactly when no consumer reads `marchMonth`, so it becomes conditional on a plan-time consumer set: a year-only loop method takes the full win (the TPC-H q7/q8/q9 shape), `year(d), month(d)` in one method correctly keeps it, and `dayofyear` (apache#64) and `trunc(d, 'YEAR')` (task 35) inherit the elision for free since both test `doy >= 306` themselves.
**The scope of that consumer set is the one place the plan was wrong**, and it is worth reading the code for. The plan said "per body", on the belief that `Slots` is planned per body method. It is not — `planSlots` receives the kernel's whole output list, and it is the group threaded into `emitBody`/`emitLaneGroup` that narrows a loop method to its own outputs. A body-scoped set therefore kept the month step in a `year(d)` loop method merely because `month(d)` was another output of the same kernel, losing the elision entirely. `"sharing the prefix leaves every loop method byte for byte as it was"` caught it on the first run. The set is now computed per lane group, beside the `emittedFragments.clear()` that has exactly that scope — which is also what makes it sound, since a fragment is re-earned in each lane group and what must hold is that every reader of `t[5]` in a group is preceded by a write of it in that group. Reading the set rather than the node being emitted is what keeps the decision order-independent under sharing.
**Measured, by minimums over five regenerations at each width:**
| | elided | kept | ratio |
|---|---|---|---|
| AVX-512, `year, null-free` | 2201.9 | 2181.7 | 1.01x |
| AVX-512, `year, mixed nulls` | 2161.7 | 2145.2 | 1.01x |
| 128-bit, `year, null-free` | 746.3 | 742.7 | 1.00x |
| 128-bit, `year, mixed nulls` | 751.9 | 750.1 | 1.00x |
Inside noise, and the sign is not stable — in two of five runs at each width the *kept* side won the mixed pair. That is the outcome the plan registered before measuring: four ops off a 43-op body at a 9 ms best time is under half a millisecond, below what the file resolves. The default ships on the step being provably dead work where it is elided and on the exhaustive sweep; the number is recorded because the repo's rule is that a performance claim traces to a committed file. The deterministic deliverable is the op count, asserted off the class file: a year-only loop body goes from 43 to 39 `IntVector` invocations.
**The regeneration also found something that is not this task.** `year, null-free` moves 1823.4 → 2166.5 M rows/s in the committed file, a fifth, and `year+month, separate` and `year(d1), year(d2)` move with it. Three things separate that from the 1.01x above: the in-run control `per-row LocalDate year`, which no Varka change touches, reads 481.1–481.6 against a committed 479.4, so the machine had not drifted; the in-run A/B puts this task's share at 1.01x; and the results file was last regenerated at `06d96642707` (task 32 step B1), after which exactly two commits touched the emitter — `71ebc645605`, task 51's removal of the per-extraction range guard, and `cb176a077eb`, task 38's column offsets, which no calendar kernel runs. By elimination it is task 51's, and the shape fits: the guard was paid once per calendar node, so a one-field kernel gains a fifth while the four-field *shared* kernel, paying it once for four tails, does not move at all (799.8 → 797.7). So **task 51 shipped a ~19% win to every single-field calendar kernel and never regenerated the parity file** — recorded in `PLAN_TASK_48.md` 9.2, generalised in `SKILLS.md`, and flagged in `PLAN_TASK_45.md`, whose baseline column was stale in every calendar row.
The epilogue's unshared `HugeMethodLimit` crossing moves from 19 outputs to 20; shared stays at 44. That is the third move of that number for the third unrelated reason, so the full ladder is re-measured and appended to `PLAN_TASK_32.md` 7.1 beside the other two rather than overwriting them.
**Also included: `PLAN_TASK_53.md`, a plan only, no code.** Velox was read alongside this lowering while the work was in progress, and its scalar decomposition turns out not to follow Hinnant's `civil_from_days` — which task 26 lowered — but Neri-Schneider 2022. One half of that paper reaches an `IntVector` lane and the other does not, and the plan separates them with an admission check of the kind task 26 established. Takeable: the month index and day of month from a single affine numerator, verified over the whole domain against the forms Varka ships (zero mismatches at all 366 values), with the one division it needs having an exact in-lane magic checked at all 65536 values — worth −2 ops on `month`, −4 on `dayofmonth`, −3 on `trunc(d, 'MONTH')`, and `year` unchanged. Not takeable: the paper's correction-free century (a 2^26 dividend against a multiplier no larger than 32) and its year step (the high half of a 64-bit product, and `VectorOperators` has no multiply-high on any lane type). Those wait for task 49's int64 lanes. Also noted there, before it is discovered: task 53 shrinks task 48's elision from four ops to two.
### Why are the changes needed?
`year(date)` is the one calendar extraction the headline corpus asks for — TPC-H q7, q8 and q9 use it and nothing else — and it was computing a month no tail read. The milestone predicted the win would mostly evaporate once the prefix was shared; making the step conditional on its actual consumers recovers it, and generalises to every other tail that does not read the month.
### Does this PR introduce _any_ user-facing change?
No. Emitted-code and plan-document changes only; the answers are unchanged, which the exhaustive sweep is what establishes.
### How was this patch tested?
Every Varka suite in catalyst and sql, at both vector widths (`-XX:MaxVectorSize=16` through the forked test JVM's `Test / javaOptions`), sweeps included.
The exhaustive sweep was extended rather than merely re-run: it now covers a year-only kernel under both sharing modes and both switch positions, over all 16,777,216 covered days, because the four-field shape keeps the month step under sharing and so never sweeps an elided prefix. `VarkaChronoSuite`'s own sweep over the same range is unchanged and green, which is the milestone's first validation criterion for this task.
New bounded tests: the 366-case identity; the op count off the class file, through a new `VarkaEmitterTestSupport.invocationCount`; order independence, where `Seq(Year, Month)` and `Seq(Month, Year)` under sharing both keep the step in the epilogue and both elide in exactly one loop method; and the unshared per-node case, where the year's own prefix elides and the month's does not.
Predictions scored in `PLAN_TASK_48.md` 9.3: 2, 3, 4 and 5 held; 1 missed in the absolute only (43 → 39 ops, not 45 → 41 — the delta was exact, the prediction counted two `VectorMask` ops the counter does not); 6 carries forward to whichever of this and apache#64 merges second.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 3, 2026
…nnot work (apache#93)
### What changes were proposed in this pull request?
Reverts the change-detection base introduced by apache#91. **It breaks every build that runs it, and it is on master, so it is breaking them now** — apache#92's `Check changes` job fails, and apache#64's will as soon as it reaches that step.
apache#91 pointed `APACHE_SPARK_REF` at the fork's own base by squash-merging `vecbricks/varka` master before squash-merging the pull request's branch. The mechanism is wrong in a way that testing the shell logic locally would not have caught:
**`git merge --squash` deliberately does not record a merge parent.** The commit it produces has no ancestry relationship to what was merged. So the second squash-merge computes its merge base against the old apache/spark commit where this fork diverged — a tree in which none of the Varka files exist — and every file both merges introduce collides:
```
CONFLICT (add/add): Merge conflict in SKILLS.md
CONFLICT (add/add): Merge conflict in .../varka/VarkaLoopEmitter.java
CONFLICT (add/add): Merge conflict in .../varka/VarkaVectorIR.java
CONFLICT (add/add): Merge conflict in .../codegen/VarkaExpressionCompiler.scala
CONFLICT (add/add): Merge conflict in .../VarkaEmitterParityBenchmark.scala
... and six more
```
Reverting rather than fixing forward, because the fix is not obvious and CI being broken is worse than CI being wasteful.
### Why are the changes needed?
**The problem apache#91 was trying to solve is real and unchanged.** Change detection diffs against apache/spark's HEAD, so on a long-lived fork every pull request looks like it changed the fork's entire divergence — 193 files, 16 of which match no module and therefore select the `root` module, which means "test everything". apache#89's ignore list removed three of nineteen such files; the rest remain.
What survives from apache#91 is the diagnostic line it added to the precondition, with its comment corrected to say what the number actually means now. Its size is still the tell, and it now names this revert so the next person does not have to rediscover the shape of the problem.
**For whoever picks this up**, two approaches remain:
- Set `APACHE_SPARK_REF` to the fetched base commit directly, with no second merge. Simple, and leaves apache/spark's drift since the fork last synced inside the diff.
- Compute the pull request's diff from real history — `merge-base` of the base branch and the head branch — and hand that to `is-changed.py`, rather than having it diff `HEAD` against a ref.
Both need verifying against a real CI run before landing, which apache#91 did not get. That is the actual lesson here: I verified apache#91 by replaying the precondition's *module math* locally against simulated merge results, which was the right check for apache#89's failure mode and the wrong one for this. The step that broke is `git`, in a repository shape I did not reproduce.
### Does this PR introduce _any_ user-facing change?
No. CI configuration only.
### How was this patch tested?
The action file parses as YAML, and the diff restores `checkout-and-sync` to exactly its pre-apache#91 content — verified by diffing against the commit before apache#91 merged. The proof it was needed is apache#92's job log, quoted above.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
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.

3 participants

@kimballa@AmplabJenkins@rxin