text:
[SPARK-15258][SQL] Nested/Chained case statements generate codegen over 64k exception

What changes were proposed in this pull request?
This PR avoid code generation for SafeProjection if the if the total number of NewInstances whose code has been generated exceeds a pre-defined threshold (400). If the total number exceeds the threshold, CodegenFallback.doGenCode is called.
The original issue was that the bytecode size in SpecificSafeProjection.apply() was more than 64KB for nested case statements with chained projection.  This PR avoids an error that bytecode size is larger than 64KB by generating smaller code with eval().
Generated code by this PR
/* 001 */
/* 002 */ public java.lang.Object generate(Object[] references) {
/* 003 */   return new SpecificSafeProjection(references);
/* 004 */ }
/* 005 */
/* 006 */ class SpecificSafeProjection extends org.apache.spark.sql.catalyst.expressions.codegen.BaseProjection {
/* 007 */
/* 008 */   private Object[] references;
/* 009 */   private MutableRow mutableRow;
/* 010 */
/* 011 */
/* 012 */
/* 013 */   public SpecificSafeProjection(Object[] references) {
/* 014 */     this.references = references;
/* 015 */     mutableRow = (MutableRow) references[references.length - 1];
/* 016 */
/* 017 */   }
/* 018 */
/* 019 */   public java.lang.Object apply(java.lang.Object _i) {
/* 020 */     InternalRow i = (InternalRow) _i;
/* 021 */     boolean isNull3 = i.isNullAt(0);
/* 022 */     InternalRow value3 = isNull3 ? null : (i.getStruct(0, 100));
/* 023 */     boolean isNull1 = false;
/* 024 */     org.apache.spark.sql.S100 value1 = null;
/* 025 */     if (!false && isNull3) {
/* 026 */       final org.apache.spark.sql.S100 value4 = null;
/* 027 */       isNull1 = true;
/* 028 */       value1 = value4;
/* 029 */     } else {
/* 030 */       // newInstance(class org.apache.spark.sql.S100)
/* 031 */       Object obj = ((Expression) references[0]).eval(i);
/* 032 */       org.apache.spark.sql.S100 value5 = (org.apache.spark.sql.S100) obj;
/* 033 */       isNull1 = false;
/* 034 */       value1 = value5;
/* 035 */     }
/* 036 */     boolean isNull8 = i.isNullAt(1);
/* 037 */     InternalRow value8 = isNull8 ? null : (i.getStruct(1, 100));
/* 038 */     boolean isNull6 = false;
/* 039 */     org.apache.spark.sql.S100 value6 = null;
/* 040 */     if (!false && isNull8) {
/* 041 */       final org.apache.spark.sql.S100 value9 = null;
/* 042 */       isNull6 = true;
/* 043 */       value6 = value9;
/* 044 */     } else {
/* 045 */       // newInstance(class org.apache.spark.sql.S100)
/* 046 */       Object obj1 = ((Expression) references[1]).eval(i);
/* 047 */       org.apache.spark.sql.S100 value10 = (org.apache.spark.sql.S100) obj1;
/* 048 */       isNull6 = false;
/* 049 */       value6 = value10;
/* 050 */     }
/* 051 */     boolean isNull13 = i.isNullAt(2);
/* 052 */     InternalRow value13 = isNull13 ? null : (i.getStruct(2, 100));
/* 053 */     boolean isNull11 = false;
/* 054 */     org.apache.spark.sql.S100 value11 = null;
/* 055 */     if (!false && isNull13) {
/* 056 */       final org.apache.spark.sql.S100 value14 = null;
/* 057 */       isNull11 = true;
/* 058 */       value11 = value14;
/* 059 */     } else {
/* 060 */       // newInstance(class org.apache.spark.sql.S100)
/* 061 */       Object obj2 = ((Expression) references[2]).eval(i);
/* 062 */       org.apache.spark.sql.S100 value15 = (org.apache.spark.sql.S100) obj2;
/* 063 */       isNull11 = false;
/* 064 */       value11 = value15;
/* 065 */     }
/* 066 */     boolean isNull18 = i.isNullAt(3);
/* 067 */     InternalRow value18 = isNull18 ? null : (i.getStruct(3, 100));
/* 068 */     boolean isNull16 = false;
/* 069 */     org.apache.spark.sql.S100 value16 = null;
/* 070 */     if (!false && isNull18) {
/* 071 */       final org.apache.spark.sql.S100 value19 = null;
/* 072 */       isNull16 = true;
/* 073 */       value16 = value19;
/* 074 */     } else {
/* 075 */       // newInstance(class org.apache.spark.sql.S100)
/* 076 */       Object obj3 = ((Expression) references[3]).eval(i);
/* 077 */       org.apache.spark.sql.S100 value20 = (org.apache.spark.sql.S100) obj3;
/* 078 */       isNull16 = false;
/* 079 */       value16 = value20;
/* 080 */     }
/* 081 */     boolean isNull23 = i.isNullAt(4);
/* 082 */     InternalRow value23 = isNull23 ? null : (i.getStruct(4, 100));
/* 083 */     boolean isNull21 = false;
/* 084 */     org.apache.spark.sql.S100 value21 = null;
/* 085 */     if (!false && isNull23) {
/* 086 */       final org.apache.spark.sql.S100 value24 = null;
/* 087 */       isNull21 = true;
/* 088 */       value21 = value24;
/* 089 */     } else {
/* 090 */       boolean isNull28 = i.isNullAt(4);
/* 091 */       InternalRow value28 = isNull28 ? null : (i.getStruct(4, 100));
/* 092 */       boolean isNull27 = isNull28;
/* 093 */       UTF8String value27 = null;
/* 094 */
/* 095 */       if (!isNull28) {
/* 096 */
/* 097 */         if (value28.isNullAt(0)) {
/* 098 */           isNull27 = true;
/* 099 */         } else {
/* 100 */           value27 = value28.getUTF8String(0);
/* 101 */         }
/* 102 */
/* 103 */       }
/* 104 */
/* 105 */       boolean isNull26 = isNull27;
/* 106 */       final java.lang.String value26 = isNull26 ? null : (java.lang.String) value27.toString();
/* 107 */       isNull26 = value26 == null;
/* 108 */       boolean isNull31 = i.isNullAt(4);
/* 109 */       InternalRow value31 = isNull31 ? null : (i.getStruct(4, 100));
/* 110 */       boolean isNull30 = isNull31;
/* 111 */       UTF8String value30 = null;
/* 112 */
/* 113 */       if (!isNull31) {
/* 114 */
/* 115 */         if (value31.isNullAt(1)) {
/* 116 */           isNull30 = true;
/* 117 */         } else {
/* 118 */           value30 = value31.getUTF8String(1);
/* 119 */         }
/* 120 */
/* 121 */       }
/* 122 */
/* 123 */       boolean isNull29 = isNull30;
/* 124 */       final java.lang.String value29 = isNull29 ? null : (java.lang.String) value30.toString();
/* 125 */       isNull29 = value29 == null;
/* 126 */       boolean isNull34 = i.isNullAt(4);
/* 127 */       InternalRow value34 = isNull34 ? null : (i.getStruct(4, 100));
/* 128 */       boolean isNull33 = isNull34;
/* 129 */       UTF8String value33 = null;
/* 130 */
/* 131 */       if (!isNull34) {
/* 132 */
/* 133 */         if (value34.isNullAt(2)) {
/* 134 */           isNull33 = true;
/* 135 */         } else {
/* 136 */           value33 = value34.getUTF8String(2);
/* 137 */         }
/* 138 */
/* 139 */       }
/* 140 */
/* 141 */       boolean isNull32 = isNull33;
/* 142 */       final java.lang.String value32 = isNull32 ? null : (java.lang.String) value33.toString();
/* 143 */       isNull32 = value32 == null;
/* 144 */       boolean isNull37 = i.isNullAt(4);
/* 145 */       InternalRow value37 = isNull37 ? null : (i.getStruct(4, 100));
/* 146 */       boolean isNull36 = isNull37;
/* 147 */       UTF8String value36 = null;
/* 148 */
/* 149 */       if (!isNull37) {
/* 150 */
/* 151 */         if (value37.isNullAt(3)) {
/* 152 */           isNull36 = true;
/* 153 */         } else {
/* 154 */           value36 = value37.getUTF8String(3);
/* 155 */         }
/* 156 */
/* 157 */       }
...
/* 1869 */       boolean isNull320 = isNull321;
/* 1870 */       final java.lang.String value320 = isNull320 ? null : (java.lang.String) value321.toString();
/* 1871 */       isNull320 = value320 == null;
/* 1872 */       boolean isNull325 = i.isNullAt(4);
/* 1873 */       InternalRow value325 = isNull325 ? null : (i.getStruct(4, 100));
/* 1874 */       boolean isNull324 = isNull325;
/* 1875 */       UTF8String value324 = null;
/* 1876 */
/* 1877 */       if (!isNull325) {
/* 1878 */
/* 1879 */         if (value325.isNullAt(99)) {
/* 1880 */           isNull324 = true;
/* 1881 */         } else {
/* 1882 */           value324 = value325.getUTF8String(99);
/* 1883 */         }
/* 1884 */
/* 1885 */       }
/* 1886 */
/* 1887 */       boolean isNull323 = isNull324;
/* 1888 */       final java.lang.String value323 = isNull323 ? null : (java.lang.String) value324.toString();
/* 1889 */       isNull323 = value323 == null;
/* 1890 */
/* 1891 */
/* 1892 */       final org.apache.spark.sql.S100 value25 = false ? null : new org.apache.spark.sql.S100(value26, value29, value32, value35, value38, value41, value44, value47, value50, value53, value56, value59, value62, value65, value68, value71, value74, value77, value80, value83, value86, value89, value92, value95, value98, value101, value104, value107, value110, value113, value116, value119, value122, value125, value128, value131, value134, value137, value140, value143, value146, value149, value152, value155, value158, value161, value164, value167, value170, value173, value176, value179, value182, value185, value188, value191, value194, value197, value200, value203, value206, value209, value212, value215, value218, value221, value224, value227, value230, value233, value236, value239, value242, value245, value248, value251, value254, value257, value260, value263, value266, value269, value272, value275, value278, value281, value284, value287, value290, value293, value296, value299, value302, value305, value308, value311, value314, value317, value320, value323);
/* 1893 */       isNull21 = false;
/* 1894 */       value21 = value25;
/* 1895 */     }
/* 1896 */
/* 1897 */
/* 1898 */     final org.apache.spark.sql.S100_5 value = false ? null : new org.apache.spark.sql.S100_5(value1, value6, value11, value16, value21);
/* 1899 */     if (false) {
/* 1900 */       mutableRow.setNullAt(0);
/* 1901 */     } else {
/* 1902 */
/* 1903 */       mutableRow.update(0, value);
/* 1904 */     }
/* 1905 */
/* 1906 */     return mutableRow;
/* 1907 */   }
/* 1908 */ }
/* 1909 */

How was this patch tested?
Added new tests

Test build #59076 has started for PR 13243 at commit e32a237.

Test build #59076 has finished for PR 13243 at commit e32a237.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59076/
Test FAILed.

Test build #59077 has started for PR 13243 at commit 59b0a76.

Test build #59077 has finished for PR 13243 at commit 59b0a76.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59077/
Test FAILed.

The fallback approach doesn't look that simple and clean, can you try split the generated code like we did in CreateExternalRow?

Test build #59098 has started for PR 13243 at commit 7c35c12.

Let me see the approach, which you proposed, later.

Test build #59098 has finished for PR 13243 at commit 7c35c12.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59098/
Test PASSed.

Test build #59109 has started for PR 13243 at commit 5bdfaa7.

Test build #59109 has finished for PR 13243 at commit 5bdfaa7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59109/
Test PASSed.

Merged build finished. Test PASSed.

mostly LGTM except some minor comments, thanks for working on it!

Test build #59118 has started for PR 13243 at commit 74d8764.

Test build #59118 has finished for PR 13243 at commit 74d8764.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59118/
Test PASSed.

Merged build finished. Test PASSed.

Test build #59122 has started for PR 13243 at commit 9cc4d41.

Test build #59122 has finished for PR 13243 at commit 9cc4d41.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59122/
Test PASSed.

Test build #59126 has started for PR 13243 at commit 6e730ca.

Test build #59126 has finished for PR 13243 at commit 6e730ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59126/
Test PASSed.

Merged build finished. Test PASSed.

Test build #59149 has started for PR 13243 at commit 6f5bda3.

Test build #59149 has finished for PR 13243 at commit 6f5bda3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59149/
Test PASSed.

Test build #59173 has started for PR 13243 at commit b92ab8c.

Test build #59173 has finished for PR 13243 at commit b92ab8c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59173/
Test PASSed.

thanks, merging to master and 2.0!

Sorry we have to revert it as it breaks scala-2.10 build

Can we just use DefinedByConstructorParams rather than using case classes?

DefinedByConstructorParams is a good idea

authors: AmplabJenkins SparkQA cloud-fan kiszk rxin


text:
Spark-16669:Adding partition prunning to Metastore statistics for better join selection.

What changes were proposed in this pull request?
Currently the metastore statistics returns the size of entire table which results in Join selection strategy to not use broadcast joins even when only a single partition from a large table is selected. This PR addresses that issue by only estimating the size of the partition by applying partition pruning during size estimation. Currently it only works with partition columns used with equality checks under AND,OR,IN Operators. If a partition column is used in any other operator, it defaults back to total table size. A config controls the behavior which is off by default.
How was this patch tested?
Unit tests added, and manually tested in local environment.

Can one of the admins verify this patch?

I think the PR is completely broken and should be recreated.

@jaceklaskowski created https://github.com/apache/spark/pull/14306. intended to open it against 2.0 branch.

source: gharchive/pull-request

repo: apache/spark

license_type: permissive

license_source: github-api


text:
[SPARK-14914][CORE]:Fix Resource not closed after using, for unit tests and example

What changes were proposed in this pull request?
This is a follow-up work of #15618.
Close file source;
For any newly created streaming context outside the withContext, explicitly close the context.
How was this patch tested?
Existing unit tests.

Test build #68379 has started for PR 15818 at commit c18af1e.

Test build #68379 has finished for PR 15818 at commit c18af1e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68379/
Test PASSed.

OK, I can merge this. This is getting confusing because 7 pull requests have been piled on one JIRA. I get that they're related and sometimes we have follow-ups but now we don't even have the changes merged into all the same branch. I think I will go back and merge all 3 related PRs into branch-2.1 to make sure we're consistent, including this one.

Merged to master; see https://github.com/apache/spark/pull/15618#issuecomment-259660817

@srowen Thanks! Next time, I will file a new JIRA if I do for the same reason.

authors: AmplabJenkins SparkQA srowen wangmiao1981


text:
[SPARK-19554][UI,YARN] Allow SHS URL to be used for tracking in YARN RM.

Allow an application to use the History Server URL as the tracking
URL in the YARN RM, so there's still a link to the web UI somewhere
in YARN even if the driver's UI is disabled. This is useful, for
example, if an admin wants to disable the driver UI by default for
applications, since it's harder to secure it (since it involves non
trivial ssl certificate and auth management that admins may not want
to expose to user apps).
This needs to be opt-in, because of the way the YARN proxy works, so
a new configuration was added to enable the option.
The YARN RM will proxy requests to live AMs instead of redirecting
the client, so pages in the SHS UI will not render correctly since
they'll reference invalid paths in the RM UI. The proxy base support
in the SHS cannot be used since that would prevent direct access to
the SHS.
So, to solve this problem, for the feature to work end-to-end, a new
YARN-specific filter was added that detects whether the requests come
from the proxy and redirects the client appropriatly. The SHS admin has
to add this filter manually if they want the feature to work.
Tested with new unit test, and by running with the documented configuration
set in a test cluster. Also verified the driver UI is used when it's
enabled.

Test build #72967 has started for PR 16946 at commit 32b8dcd.

Test build #72967 has finished for PR 16946 at commit 32b8dcd.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class YarnProxyRedirectFilter extends Filter with Logging

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72967/
Test PASSed.

@tgravescs pinging you since this is all YARN-side (doesn't really touch the UI).

Test build #73007 has started for PR 16946 at commit d675717.

Test build #73007 has finished for PR 16946 at commit d675717.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73007/
Test PASSed.

trying to get some eyes on this: @squito

Test build #73242 has started for PR 16946 at commit 5aef8eb.

On vacation back next Monday and will review.

Test build #73242 has finished for PR 16946 at commit 5aef8eb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73242/
Test PASSed.

Test build #73287 has started for PR 16946 at commit b380a85.

Test build #73287 has finished for PR 16946 at commit b380a85.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73287/
Test PASSed.

Test build #73296 has started for PR 16946 at commit e0df6a5.

Test build #73296 has finished for PR 16946 at commit e0df6a5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73296/
Test PASSed.

Merging to master.
@tgravescs we can address any feedback you might have when you're back.

authors: AmplabJenkins SparkQA tgravescs vanzin


text:
[SPARK-13669][SPARK-20898][Core] Improve the blacklist mechanism to handle external shuffle service unavailable situation

What changes were proposed in this pull request?
Currently we are running into an issue with Yarn work preserving enabled + external shuffle service.
In the work preserving enabled scenario, the failure of NM will not lead to the exit of executors, so executors can still accept and run the tasks. The problem here is when NM is failed, external shuffle service is actually inaccessible, so reduce tasks will always complain about the “Fetch failure”, and the failure of reduce stage will make the parent stage (map stage) rerun. The tricky thing here is Spark scheduler is not aware of the unavailability of external shuffle service, and will reschedule the map tasks on the executor where NM is failed, and again reduce stage will be failed with “Fetch failure”, and after 4 retries, the job is failed. This could also apply to other cluster manager with external shuffle service.
So here the main problem is that we should avoid assigning tasks to those bad executors (where shuffle service is unavailable). Current Spark's blacklist mechanism could blacklist executors/nodes by failure tasks, but it doesn't handle this specific fetch failure scenario. So here propose to improve the current application blacklist mechanism to handle fetch failure issue (especially with external shuffle service unavailable issue), to blacklist the executors/nodes where shuffle fetch is unavailable.
How was this patch tested?
Unit test and small cluster verification.

Test build #73668 has started for PR 17113 at commit a90b23d.

Test build #73668 has finished for PR 17113 at commit a90b23d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73668/
Test PASSed.

can you clarify the situations you are seeing issues? What happened to the NM in this case.  If you have work preserving restart I would think this would actually cause you more problems.  The NM could temporarily be down during rolling upgrade and if you blacklist it, it won't be used for a long time.
We have seen issues with TEZ and MR where blacklisting on fetch failures caused more issues then it solved.  Most of the fetch failures were transient issues and it caused way more things to be rerun then was actually needed.
This is why we explicitly left it out of the blacklisting feature.  See the design doc here in the jira https://issues.apache.org/jira/browse/SPARK-8425.
I didn't have a chance to do a full review but you seem to be blacklisting an executor, what executor is this blacklisting?  It looks like you are immediately blacklisting on any fetch failure rather then allowing configurable number?

"Current Spark's blacklist mechanism": please be more precise. The most recent released version of Spark, 2.1.0, does not include a lot of recent changes to blacklisting (mostly https://github.com/apache/spark/commit/93cdb8a7d0f124b4db069fd8242207c82e263c52). Are the problems you are describing fully explored with the master branch of Spark?

@tgravescs , the main scenario is external shuffle service unavailable scenario, this could be happened in working preserving + NM failure situation. Also like Mesos + external standalone shuffle service could introduce this issue. In scenarios like rolling upgrade I agreed that NM unavailability is short and this issue could be self-recoverable. One scenario I'm simulating is NM failure. In my test, when NM is failed, RM will detect this failure after 10 minutes by default, before that executors on that NM can still serve the tasks, and Spark doesn't blacklist these containers, so re-issued tasks could still be failed.
FetchFailed will immediately abort the running stage and re-issue parent stage, configurations like failed task number per stage may not be so useful, so my thinking is to backlist these executors/nodes immediately after fetch failure.
This proposal may have many problems for different scenario, that's why I opened here for comments. If you don't think it is necessary to fix then I could close it.
@markhamstra this patch is targeted to master branch and all the investigations and changes is based on master branch.

@jerryshao  are you actually seeing issues with this on real customer/production jobs?  How often? NM failure for us is very rare.  I'm not familiar with how mesos would fail differently, the shuffle service there is started as a separate service correct?
We would definitely need to make sure that Spark's retry before really returning fetch failure are good enough to handle the cases with rolling upgrade or intermittent issues with the shuffle, but with our defaults of 3 retries at 5 seconds each not sure that would cover it.
If people are seeing a need for it and actual issues with it then I'm ok with the idea as long as we make it configurable to turn it off.  I'm not sure on the blacklist after one fetch failure either.  It would seem better to only blacklist after a couple of things have gotten the failure that way you would have a better confidence it was really an issue with the node you are fetching from.
cc @squito

@tgravescs , thanks a lot for your comments.
Actually the issue here is a simulated one from my test cluster, I didn't get an issue report from real customers.
Yes, in most of the cases Shuffle fetch failure is transient and could be recovered through retry, but for some small jobs this gap between failure and recovery is enough for the job to be failed with retry.
The difficult thing here is that for fetch failure, Spark immediately abort the stage without any retry of the tasks, so unlike normal failures of tasks, we may have no chance to monitor several fetch failures until we decide to blacklist it. That's why in this PR I immediately blacklist the executors/nodes in the application level after fetch failure. The solution is too strict that it will also blacklist some transient failure executors, that's what I mainly concerned about.
So here looking for any comments, greatly appreciated.

So I looked at this a little more.  I'm more ok with this since Spark doesn't actually invalidate the shuffle output. You are basically just trying to stop new tasks from running on the executors already on that host. Its either going to just blacklist those or kill them if you have that feature on.
Part of the reason we left it off to begin with was again we didn't want to blacklist on the transient ones so we wanted to wait to see if it was truly an issue in real life.  if you do put this in I would like it configurable off until we have more data as to if its really a problem users see.
Spark does immediately abort the stage but it doesn't kill the running tasks, so if other tasks fetch failure before it can rerun the map task the scheduler knows about them, but that is very timing dependent.

Spark does immediately abort the stage but it doesn't kill the running tasks

Whether running tasks are interrupted on stage abort or not depends on the state of a config boolean -- and ideally we'd like to get to the point where we can confidently set that config so that running tasks are interrupted when the associated job or stage dies.

@markhamstra given the impact interruption has on lower layer libraries which dont handle it well (iirc hdfs ?), we probably will not set it to true even if spark code is robust,

@mridulm Correct, turning task interruption on by default is not so much a matter of Spark itself handling it well as it is a possible (though not completely known) issue with lower layer libraries not handling interruption well. The original concern with HDFS is likely fixed now, but there are similar concerns with Cassandra and other libraries. Logically, we'd like to interrupt Tasks when associated Jobs or Stages are killed in the DAGScheduler. In practice, nobody knows right now how to do that safely in all circumstances, so the default is to not attempt to interrupt the tasks.

Whether running tasks are interrupted on stage abort or not depends on the state of a config boolean -- and ideally we'd like to get to the point where we can confidently set that config so
that running tasks are interrupted when the associated job or stage dies.

@markhamstra which config are you referring to?

I think killing tasks is only applicable in different scenarios, eg. if the job fails.  Currently, spark does not cancel running tasks when the stage fails due to a fetch failure.  The taskset is marked as a zombie, but running tasks are left alone.

@tgravescs At the config level, it is spark.job.interruptOnCancel or SparkContext.SPARK_JOB_INTERRUPT_ON_CANCEL, which then gets passed around as a boolean -- e.g. shouldInterruptThread.

@squito Correct, we really only try to kill running tasks currently on job failure (and if the config setting allows it); but there is the long-standing "TODO: Cancel running tasks in the stage" in case FetchFailed of DAGScheduler#handleTaskCompletion, which has languished as a TODO both because resolving it would require us both to make Spark itself handle task interruption in the fetch failure case and to deal with the same issues preventing us from making task interruption the default even for job cancelation.
All I'm really saying is that we shouldn't design in a hard requirement that tasks cannot be interrupted (on job cancelation, fetch failure or some other event), because we'd often really like to be able to kill running tasks -- even though we don't know quite how to do that safely right now.

@markhamstra Completely agree, I would love to see this enabled by default. For example, I really hate to see speculative tasks continuing to run when the taskset has completed (for example) - used to make it necessary to overprovision to ensure subsequent taskset has all tasks running from get go.

I was not talking about designing this around the killing task part of this, other then in reference to being able to count the # of fetch failures before triggering the blacklisting, but I think that would have to be handled across stages right now.  My main concern with this pr is invalidating the shuffle outputs.  I do also believe that fetch failures could be handled better but that is more being discussed in https://github.com/apache/spark/pull/17088.
I do also agree with you that the task killing part should be fixed.

Test build #74234 has started for PR 17113 at commit 7ba0623.

@tgravescs , I just added a configuration to turn off this feature by default.
Do you have any further comments on it?

Test build #74234 has finished for PR 17113 at commit 7ba0623.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/74234/
Test PASSed.

Merged build finished. Test PASSed.

sorry haven't had a chance to get to this to do full review, hopefully tomorrow.

Another thing I thought about as I was reviewing this -- spark currently assumes that a fetchfailure is always the fault of the source, never the destination. I almost wonder if we should count it against both, with some sensible heuristic for looking at a collection of failures and deciding who is at fault.

I think this makes sense to try to tell the difference and improve our logic there.

Perhaps we should think of a better way to choose the right behavior.

Does this mean you don't want to go with this approach?  I'm actually not sure this is a huge change.   Its a decent change in behavior but for the cases where nodes really do go down this could help a lot.  I think Spark definitely doesn't handle this case well now.
Sorry again I haven't done a full review been trying to think the entire fetch failure scenarios through and have just been busy with other things.
One downside to adding the config BLACKLIST_FETCH_FAILURE_ENABLED limits us on possibly changing this functionality to say only blacklist on multiple fetch failures.  We could track fetch failures across stage attempts and say only after it fails X number of tasks which could be across stage attempts do we blacklist it.  I guess its marked as experimental so its a bit more ok for us to change it.   Perhaps X isn't just failed tasks but failed tasks from a % of different hosts.  You could potentially use that also to determine if the source is bad or the destination.  If many across hosts have failed then you expect the source is bad, if its just one destination then perhaps that is bad.  Still thinking this through.

sorry i was vague -- I'm saying I'm ok with this as long as its (a) off by default and (b) experimental so we can change it around (which it is).

Test build #75095 has started for PR 17113 at commit f6cc47e.

Test build #75095 has finished for PR 17113 at commit f6cc47e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75095/
Test FAILed.

Jenkins, retest this please.

Test build #75152 has started for PR 17113 at commit f6cc47e.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75152/
Test FAILed.

sorry for the delay on this we have been having some discussion about scheduler changes and the fetch failure handling in the scheduler.  Since this is related holding off on this.

Thanks @tgravescs , no problem.

@jerryshao  sorry my delay on this, we have rough design what we want to do for future changes but I think those are going to take a while and in the mean time I think this is a useful addition as we have been seeing issues this would have helped.
Could you bring this up to date and address the comments?

Thanks @tgravescs , I will update the code soon.

Test build #77344 has started for PR 17113 at commit 44c7108.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77344/
Test FAILed.

Jenkins, retest this please.

Test build #77346 has started for PR 17113 at commit 44c7108.

Test build #77346 has finished for PR 17113 at commit 44c7108.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77346/
Test FAILed.

Test build #77367 has started for PR 17113 at commit 44c7108.

Test build #77367 has finished for PR 17113 at commit 44c7108.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77367/
Test PASSed.

I'm curious did you test the killing part on an actual yarn job?  I was trying it on master and I don't think it works at all due to the way its passing allocation client.  Its a separate issue then this just wondering if you saw same thing

@squito  just double checking, are you ok with this change and did you have any comments?

Sorry @tgravescs I didn't test executor killing in real cluster. There has bug in it, so I pushed a commit to fix it. Thanks for your reviewing.

Test build #77446 has started for PR 17113 at commit 524fbfc.

Test build #77446 has finished for PR 17113 at commit 524fbfc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77446/
Test PASSed.

so unfortunately I haven't actually been seeing this. You can see with external shuffle is something happens to the NM and it does cause job failure.   NM crashes for OOM, something else kills it accidentally, etc.  In those cases the executors are still running fine and map tasks won't fail if its already registered, then you get fetch failure again and iterate until job failure. An alternative to this might be to have the executor re-ping the external shuffle service before starting a new task and have it fail at that point.
We could also add in logic to only blacklist affect certain number of failures, but I don't see this as much different as SPARK-19753 where its going to invalidate immediately.

then you get fetch failure again and iterate until job failure

At first I was thinking the node goes bad, but you first detect it via fetch failures -- in that case, you wouldn't need this, when the tasks get re-run on the bad node, they'd fail, and the node would still get blacklisted.  But you're saying the issue is if the node is actually totally healthy, so tasks can still write shuffle data, but the external shuffle service becomes unusable.
I don't think of losing all the shuffle data to be as aggressive as blacklisting the node, but maybe you're right, its pretty similar.
I guess I'm OK with this, though I'm still not sure when I'd tell users to turn this on.

Test build #77575 has started for PR 17113 at commit 9a14105.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77575/
Test FAILed.

Jenkins, retest this please.

Test build #77582 has started for PR 17113 at commit 9a14105.

Test build #77582 has finished for PR 17113 at commit 9a14105.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77582/
Test PASSed.

please add jira SPARK-20898 to the description since fixing that here

Test build #77622 has started for PR 17113 at commit 3cf9cfd.

Test build #77622 has finished for PR 17113 at commit 3cf9cfd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77622/
Test PASSed.

changes LGTM.
@squito  did you have any further comments?

I'm fine with this, just a couple asks:

is there a test for SPARK-20898?  I know at some point in the development of killing blacklisted executors we tested on yarn, I'm really disappointed (in myself) for merging it though it didn't actually work.  It would be nice to have a regression test.  Maybe that would be a huge pain to do, just wanted to see if you thought about it.

Can you update the jira description for SPARK-13669 now?  I think a lot has changed since that was initially opened.  Also IIUC, we don't have any particular advice on when users should enable this at the moment, its just an escape hatch you'd like to leave in place?  If there is any advice, putting it in the jira would also make sense.

Hi @squito ,
For the 1st point, I tested manually in real cluster, but I'm not sure how to make it happen in UT, if you think it is necessary to add a UT about this issue, then I will think about how to address it.
For the 2nd point, I will update the JIRA description, thanks for the remainder.

Jenkins, test this please

Test build #78301 has started for PR 17113 at commit 3cf9cfd.

Test build #78301 has finished for PR 17113 at commit 3cf9cfd.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78301/
Test FAILed.

retest this please

Test build #78334 has started for PR 17113 at commit 3cf9cfd.

Test build #78334 has finished for PR 17113 at commit 3cf9cfd.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

these failures definitely look unrelated. I'll kick once more to try to get clean run.

Jenkins, test this please

Jenkins, retest this please.

Test build #78431 has started for PR 17113 at commit 3cf9cfd.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78431/
Test FAILed.

Emmm... the failure is irrelevant, retest this please.

Test build #78446 has started for PR 17113 at commit 3cf9cfd.

Test build #78446 has finished for PR 17113 at commit 3cf9cfd.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78446/
Test FAILed.

Looks like Jenkins is not so stable.

Jenkins, retest this please.

Test build #78457 has started for PR 17113 at commit 3cf9cfd.

Test build #78457 has finished for PR 17113 at commit 3cf9cfd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/78457/
Test PASSed.

+1 finally got a clean build, will merge to master

authors: AmplabJenkins SparkQA jerryshao jiangxb1987 markhamstra mridulm squito tgravescs


text:
[SPARK-21990][SQL] QueryPlanConstraints misses some constraints that can be recursively inferred

What changes were proposed in this pull request?
When I inspected the latest change of SPARK-21979, I found we could miss few constraints that can be recursively inferred.
E.g. consider constraints a === b && b === c && a === 1 && b === 5. Currently we only infer a === c && b === 1 && c === 5, but c === 1 can be also inferred from newly inferred constraint a === c.
How was this patch tested?
Added test.

Test build #81711 has started for PR 19216 at commit d76f8f0.

Test build #81711 has finished for PR 19216 at commit d76f8f0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81711/
Test FAILed.

Merged build finished. Test FAILed.

Test build #81720 has started for PR 19216 at commit 4e85e5f.

Test build #81720 has finished for PR 19216 at commit 4e85e5f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81720/
Test PASSed.

Hi @viirya ,
could you find a better example?
The one in the description/test case should be optimized to False.

@gengliangwang Sorry, what the meaning it should be optimized to False?

 a === b && a === 1 && b === 5

The result is always False.

@gengliangwang Thanks. I'm modifying the test case.

@gengliangwang How about the modified one in the description?

Test build #81749 has started for PR 19216 at commit e4cffda.

Thanks.
I run the test case with the rule "InferFiltersFromConstraints", the result has c > 5.
The rule will be executed multiple times.

@gengliangwang Yeah, I see. InferFiltersFromConstraints will construct new Filter from inferred constraints and c > 1 will be inferred in the new Filter. I'm trying to see if there can be an exception. If no, I think it's safe and I can close this one.

@gengliangwang I think it should be safe. Let me close this now. Thanks.

Test build #81749 has finished for PR 19216 at commit e4cffda.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81749/
Test PASSed.

I'm thinking more about this.
Actually I think we shouldn't rely on an optimization rule to produce correct constraints. If there is a logical plan other than Filter that can get its material of constraints from outside (maybe data source or others) in the future, we won't infer the correct constraints.
But currently looks like it doesn't cause any trouble.

Yes, I agree. But constraints is used in filter inference only, and it works now. The code changes in this PR make the logic complex.
I think we should make the logic simpler, if we decide to do this.

Yeah, as it doesn't cause any trouble now, I don't bother to re-open it.
Btw, another bad to reply on the optimization rule to produce correct constraints is the extra cost introduced by re-constructing the whole query plan tree, and the re-inferring of constraints in Filter. Instead of this, inferring the correct constraints in one pass should be more efficient.

authors: AmplabJenkins SparkQA gengliangwang viirya


text:
[SPARK-22473][FOLLOWUP][TEST] Remove deprecated Date functions

What changes were proposed in this pull request?
#19696 replaced the deprecated usages for Date and Waiter, but a few methods were missed. The PR fixes the forgotten deprecated usages.
How was this patch tested?
existing UTs

Test build #84425 has started for PR 19875 at commit 424e471.

yes @HyukjinKwon , you are 100% right, sorry for this error.

Test build #84425 has finished for PR 19875 at commit 424e471.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84425/
Test PASSed.

Merged to master

authors: AmplabJenkins SparkQA mgaido91 srowen


text:
[SPARK-23313][DOC] Add a migration guide for ORC

What changes were proposed in this pull request?
This PR adds a migration guide documentation for ORC.

How was this patch tested?
N/A.

Test build #86969 has started for PR 20484 at commit 20f99c6.

cc @gatorsmile and @cloud-fan .

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/509/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/510/
Test PASSed.

Merged build finished. Test PASSed.

Test build #86970 has started for PR 20484 at commit 1bb23ef.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/511/
Test PASSed.

Test build #86969 has finished for PR 20484 at commit 20f99c6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86969/
Test PASSed.

Merged build finished. Test PASSed.

Test build #86971 has started for PR 20484 at commit df08899.

Test build #86970 has finished for PR 20484 at commit 1bb23ef.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86970/
Test PASSed.

Merged build finished. Test PASSed.

Test build #86971 has finished for PR 20484 at commit df08899.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86971/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/513/
Test PASSed.

Merged build finished. Test PASSed.

Test build #86973 has started for PR 20484 at commit 239714a.

Test build #86973 has finished for PR 20484 at commit 239714a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86973/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/517/
Test PASSed.

Test build #86977 has started for PR 20484 at commit fc5b395.

Test build #86978 has started for PR 20484 at commit 7b3b0a4.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/519/
Test PASSed.

Test build #86977 has finished for PR 20484 at commit fc5b395.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Test build #86978 has finished for PR 20484 at commit 7b3b0a4.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86978/
Test FAILed.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86977/
Test FAILed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/524/
Test PASSed.

Merged build finished. Test PASSed.

Test build #86984 has started for PR 20484 at commit cb149f2.

Test build #86984 has finished for PR 20484 at commit cb149f2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86984/
Test PASSed.

@dongjoon-hyun Do our native readers respect Hive confs? If not, we need to clearly document it. I think this is a common question from the existing users of ORC readers, since they are respected in the prior versions.

Actually, I handed over all confs to ORC library. The following is all supported ORC configuration.

https://github.com/apache/orc/blob/master/java/core/src/java/org/apache/orc/OrcConf.java

Please note that the confs are interpreted by both Hive Conf Name and ORC Conf Names. Yesterday, @tgravescs asked about hive.exec.orc.split.strategy. For now, it's not recognized by ORC library according to OrcConf.java.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/547/
Test PASSed.

Merged build finished. Test PASSed.

Test build #87011 has started for PR 20484 at commit 436c0f4.

Test build #87011 has finished for PR 20484 at commit 436c0f4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87011/
Test PASSed.

Test build #87102 has started for PR 20484 at commit 354a525.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/624/
Test PASSed.

Test build #87102 has finished for PR 20484 at commit 354a525.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87102/
Test PASSed.

@dongjoon-hyun What we need to add in the migration guide is to help the existing ORC users migrate their existing queries that are built on ORC tables.
The Hive ORC confs can be set in the hiveconf? the table properties? the serde properties (our options)? Are they still working after we implicitly/automatically convert them to use our new ORC reader? If not, we need to documents them. This also answers the questions by @tgravescs in the JIRA.
I knew there are a few conf-related bugs. It might be too late to fix them in the RC of 2.3 release, but we have to identify and document them. Thanks for your works!

I understand the situation, but I hope this will not introduce a regression due to the change of default value of convertMetastoreOrc. I created another PR, https://github.com/apache/spark/pull/20522.

With a combination of #20517 and #20522 , (1) configuration names, (2) table properties, (3) serde properties, (4) convertMetastoreOrc/Parquet=true are tested and fixed.
The only remaining thing looks like Hive ORC confs.

How about users setting the ORC-related confs in HiveConf configuration files? Are they still effective in Hive?

I'm looking at that.
BTW, can we finish table property issue in Apache Spark 2.3 at #20522 ?
Given that users already experience this for STORED AS PARQUET Hive table until now. Is it okay to mention that ORC table will do the same with Parquet table from Spark 2.3?

For the HiveConf configuration file, I tested manually. It works correctly.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/648/
Test PASSed.

Test build #87136 has started for PR 20484 at commit d259d66.

Test build #87136 has finished for PR 20484 at commit d259d66.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87136/
Test PASSed.

Are they still effective in Hive?

I just want to confirm whether all the Hive readers work fine. Could you add a test case like what we did in CliSuite?

@dongjoon-hyun This is still a regression to the existing Hive ORC users. cc @cloud-fan @sameeragarwal Maybe we should fix it before the release?

Just FYI, https://github.com/apache/spark/pull/20536 is reverting the conf convertMetastoreOrc back false. However, we still can turn it on by default in 2.3 after we fix the regression.
Thanks!

I see. I removed spark.sql.hive.convertMetastoreOrc and Hive ORC table stuff from this PR accordingly. We can add that later if we fix the regresson of convertMetastoreOrc/Parquet.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/678/
Test PASSed.

Test build #87177 has started for PR 20484 at commit 40c8e02.

why did you remove the bit about the orc configs?

Test build #87177 has finished for PR 20484 at commit 40c8e02.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87177/
Test PASSed.

Oh, I thought in this way, @tgravescs .

For ORC files, the parameter names (ORC/Hive) are the same as we see https://github.com/apache/spark/pull/20517 .
For Hive tables, we don't need a migration because we didn't touch it after #20536 . Although users turn on by manually, the table properties of Hive ORC tables are ignored completely without #20522 .

ok

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/817/
Test PASSed.

Test build #87342 has started for PR 20484 at commit 59e957a.

Test build #87342 has finished for PR 20484 at commit 59e957a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87342/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/823/
Test PASSed.

Test build #87351 has started for PR 20484 at commit 6136d25.

Test build #87353 has started for PR 20484 at commit 8ae87fc.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/825/
Test PASSed.

LGTM except the above comment.

Test build #87351 has finished for PR 20484 at commit 6136d25.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87351/
Test PASSed.

Test build #87353 has finished for PR 20484 at commit 8ae87fc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87353/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/826/
Test PASSed.

Test build #87354 has started for PR 20484 at commit 6887d19.

Test build #87354 has finished for PR 20484 at commit 6887d19.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87354/
Test PASSed.

Thanks! Merged to master and 2.3.

Thank you, @gatorsmile , @tgravescs , @felixcheung .

authors: AmplabJenkins SparkQA dongjoon-hyun gatorsmile tgravescs


text:
[SPARK-31030] [DOCS] [FOLLOWUP] Replace HTML Table by Markdown Table

What changes were proposed in this pull request?
This PR is to clean up the markdown file in datetime-pattern page.

Replace HTML table by MD table

Why are the changes needed?
Make the doc cleaner and easily editable by MD editors.
Does this PR introduce any user-facing change?
NO
How was this patch tested?
Before

After

cc @huaxingao Could you help fix the other similar cases?
I am using this tool to do the conversion: https://jmalarcon.github.io/markdowntables/

Test build #122104 has started for PR 28415 at commit fc18275.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26778/
Test PASSed.

Test build #122104 has finished for PR 28415 at commit fc18275.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122104/
Test PASSed.

thanks, merging to master/3.0!

authors: AmplabJenkins SparkQA cloud-fan gatorsmile


text:
[SPARK-31623][SQL][TESTS] Benchmark rebasing of INT96 and TIMESTAMP_MILLIS timestamps in read/write

What changes were proposed in this pull request?
Add new benchmarks to DateTimeRebaseBenchmark for reading/writing timestamps of INT96 and TIMESTAMP_MICROS column types. Here are benchmark results for reading timestamps after 1582 year with default settings (rebasing is off for TIMESTAMP_MICROS/TIMESTAMP_MILLIS,  and rebasing on for INT96):

timestamp type
vectorized off (ns/row)
vectorized on (ns/row)

TIMESTAMP_MICROS
160.1
50.2

INT96
215.6
117.8

TIMESTAMP_MILLIS
159.9
60.6

Why are the changes needed?
To compare default timestamp type TIMESTAMP_MICROS with other types in the case if an user decides to switch on them.
Does this PR introduce any user-facing change?
No
How was this patch tested?
By running the benchmarks via:
SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.DateTimeRebaseBenchmark"

in the environment:

Item
Description

Region
us-west-2 (Oregon)

Instance
r3.xlarge

AMI
ubuntu/images/hvm-ssd/ubuntu-bionic-18.04-amd64-server-20190722.1 (ami-06f2f779464715dc5)

Java
OpenJDK 64-Bit Server VM 1.8.0_252-8u252 and OpenJDK 64-Bit Server VM 11.0.7+10

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26846/
Test PASSed.

Test build #122174 has started for PR 28431 at commit b98c3d0.

Test build #122174 has finished for PR 28431 at commit b98c3d0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122174/
Test PASSed.

@cloud-fan @HyukjinKwon Please, take a look at this.

Test build #122263 has started for PR 28431 at commit 07ac3d4.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26933/
Test PASSed.

Test build #122264 has started for PR 28431 at commit 283b554.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26934/
Test PASSed.

Test build #122265 has started for PR 28431 at commit 0c9f9d5.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26935/
Test PASSed.

Test build #122264 has finished for PR 28431 at commit 283b554.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122264/
Test FAILed.

Test build #122263 has finished for PR 28431 at commit 07ac3d4.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122263/
Test FAILed.

jenkins, retest this, please

Test build #122276 has started for PR 28431 at commit 0c9f9d5.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26946/
Test PASSed.

Test build #122265 has finished for PR 28431 at commit 0c9f9d5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122265/
Test PASSed.

Test build #122276 has finished for PR 28431 at commit 0c9f9d5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/122276/
Test PASSed.

thanks, merging to master/3.0!

authors: AmplabJenkins MaxGekk SparkQA cloud-fan


text:
[SPARK-32852][SQL][Doc][Followup] Revise the documentation of spark.sql.hive.metastore.jars

What changes were proposed in this pull request?

This is a follow-up for https://github.com/apache/spark/pull/29881.
It revises the documentation of the configuration spark.sql.hive.metastore.jars.
Why are the changes needed?

Fix grammatical error in the doc.
Also, make it more clear that the configuration is effective only when spark.sql.hive.metastore.jars is set as path
Does this PR introduce any user-facing change?

No
How was this patch tested?

Just doc changes.

cc @AngersZhuuuu @cloud-fan

Test build #131265 has started for PR 30407 at commit 8abc00f.

Test build #131264 has started for PR 30407 at commit b07b135.

Test build #131267 has started for PR 30407 at commit 93c7d10.

Test build #131269 has started for PR 30407 at commit d7f040c.

Test build #131267 has finished for PR 30407 at commit 93c7d10.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test build #131264 has finished for PR 30407 at commit b07b135.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131267/
Test FAILed.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131264/
Test FAILed.

Test build #131265 has finished for PR 30407 at commit 8abc00f.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131265/
Test FAILed.

Test build #131269 has finished for PR 30407 at commit d7f040c.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131269/
Test FAILed.

Test build #131272 has started for PR 30407 at commit 45922bd.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35867/

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35868/

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35868/

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35868/
Test PASSed.

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35867/

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35867/
Test PASSed.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35870/

Test build #131272 has finished for PR 30407 at commit 45922bd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131272/
Test PASSed.

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35870/

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35870/
Test PASSed.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35872/

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35874/

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35872/

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35872/
Test PASSed.

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35874/

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35874/
Test PASSed.

@dongjoon-hyun @AngersZhuuuu @cloud Thanks for the review

Merging to master

authors: AmplabJenkins SparkQA gengliangwang


text:
[SPARK-34547][SQL] Only use metadata columns for resolution as last resort

What changes were proposed in this pull request?
Today, child expressions may be resolved based on "real" or metadata output attributes. We should prefer the real attribute during resolution if one exists.
Why are the changes needed?
Today, attempting to resolve an expression when there is a "real" output attribute and a metadata attribute with the same name results in resolution failure. This is likely unexpected, as the user may not know about the metadata attribute.
Does this PR introduce any user-facing change?
Yes. Previously, the user would see an error message when resolving a column with the same name as a "real" output attribute and a metadata attribute as below:
org.apache.spark.sql.AnalysisException: Reference 'index' is ambiguous, could be: testcat.ns1.ns2.tableTwo.index, testcat.ns1.ns2.tableOne.index.; line 1 pos 71
at org.apache.spark.sql.catalyst.expressions.package$AttributeSeq.resolve(package.scala:363)
at org.apache.spark.sql.catalyst.plans.logical.LogicalPlan.resolveChildren(LogicalPlan.scala:107)

Now, resolution succeeds and provides the "real" output attribute.
How was this patch tested?
Added a unit test.

Test build #135482 has started for PR 31654 at commit 1c5ab03.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40062/

Test build #135482 has finished for PR 31654 at commit 1c5ab03.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135482/

thanks, merging to master/3.1!

authors: AmplabJenkins SparkQA cloud-fan karenfeng


text:
[SPARK-34599][SQL]Fix the issue that INSERT INTO OVERWRITE doesn't support partition columns containing dot for DSv2

What changes were proposed in this pull request?
ResolveInsertInto.staticDeleteExpression should use UnresolvedAttribute.quoted to create the delete expression so that we will treat the entire attr.name as a column name.
Why are the changes needed?
When users use dot in a partition column name, queries like INSERT OVERWRITE $t1 PARTITION (`a.b` = 'a') (`c.d`) VALUES('b') is not working.
Does this PR introduce any user-facing change?
Without this test, the above query will throw
[info]   org.apache.spark.sql.AnalysisException: cannot resolve '`a.b`' given input columns: [a.b, c.d];
[info] 'OverwriteByExpression RelationV2[a.b#17, c.d#18] default.tbl, ('a.b <=> cast(a as string)), false
[info] +- Project [a.b#19, ansi_cast(col1#16 as string) AS c.d#20]
[info]    +- Project [cast(a as string) AS a.b#19, col1#16]
[info]       +- LocalRelation [col1#16]

With the fix, the query will run correctly.
How was this patch tested?
The new added test.

Test build #135678 has started for PR 31713 at commit 8468b7f.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40260/

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40260/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40260/

Test build #135678 has finished for PR 31713 at commit 8468b7f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135678/

thanks, merging to master/3.1!

authors: AmplabJenkins SparkQA cloud-fan zsxwing


text:
[SPARK-35058][SQL] Group exception messages in hive/client

What changes were proposed in this pull request?
This PR group exception messages in sql/hive/src/main/scala/org/apache/spark/sql/hive/client.
Why are the changes needed?
It will largely help with standardization of error messages and its maintenance.
Does this PR introduce any user-facing change?
No. Error messages remain unchanged.
How was this patch tested?
No new tests - pass all original tests to make sure it doesn't break any existing behavior.

Test build #139270 has started for PR 32763 at commit 7df0c2a.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43794/

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43794/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43794/

Test build #139270 has finished for PR 32763 at commit 7df0c2a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

ping @allisonwang-db

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139270/

cc @cloud-fan

Test build #139363 has started for PR 32763 at commit 6f75f27.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43885/

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43885/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43885/

Test build #139363 has finished for PR 32763 at commit 6f75f27.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139363/

Test build #139493 has started for PR 32763 at commit 83c1ab8.

Test build #139493 has finished for PR 32763 at commit 83c1ab8.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44017/

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44017/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44017/

retest this please

Test build #139522 has started for PR 32763 at commit 83c1ab8.

Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44047/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44047/

Test build #139522 has finished for PR 32763 at commit 83c1ab8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

ping @cloud-fan

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139522/

thanks, merging to master!

@allisonwang-db @cloud-fan  Thanks for review.

authors: AmplabJenkins SparkQA beliefer cloud-fan


text:
[SPARK-35895][SQL] Support subtracting Intervals from TimestampWithoutTZ

What changes were proposed in this pull request?

Support the following operation:

TimestampWithoutTZ - Year-Month interval

The following operation is actually supported in https://github.com/apache/spark/pull/33076/. This PR is to add end-to-end tests for them:

TimestampWithoutTZ - Calendar interval
TimestampWithoutTZ - Daytime interval

Why are the changes needed?

Support subtracting all 3 interval types from a timestamp without time zone
Does this PR introduce any user-facing change?

No, the timestamp without time zone type is not release yet.
How was this patch tested?

Unit tests

Test build #140323 has started for PR 33086 at commit 55ce2b6.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44855/

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44855/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/44855/

Test build #140323 has finished for PR 33086 at commit 55ce2b6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/140323/

The build failure by Scala 2.13:
[error] /home/runner/work/spark/spark/core/src/main/scala/org/apache/spark/SparkContext.scala:428:30: type mismatch;
[error]  found   : Seq[String] (in scala.collection)
[error]  required: Seq[String] (in scala.collection.immutable)
[error]     _jars = Utils.getUserJars(_conf)

is not related to the changes, I think.

+1, LGTM. Merging to master.
Thank you, @gengliangwang .

authors: AmplabJenkins MaxGekk SparkQA gengliangwang


text:
[SPARK-37153][PYTHON] Inline type hints for python/pyspark/profiler.py

What changes were proposed in this pull request?
Inline type hints for python/pyspark/profiler.py
Why are the changes needed?
We can take advantage of static type checking within the functions by inlining the type hints.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Existing tests

Test build #145693 has started for PR 34731 at commit b947cc4.

Test build #145693 has finished for PR 34731 at commit b947cc4.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Let's get https://github.com/apache/spark/pull/34685 done first.

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50163/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145693/

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50163/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50163/

Test build #145743 has started for PR 34731 at commit 48e355e.

Test build #145745 has started for PR 34731 at commit 27207aa.

Test build #145743 has finished for PR 34731 at commit 48e355e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test build #145745 has finished for PR 34731 at commit 27207aa.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145745/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145743/

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50213/

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50216/

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50213/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50213/

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50216/

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50216/

@dchvn Maybe could you rebase onto or merge master branch and see what happens?
@zero323 Now that #34363 has been merged. Could you give it another pass?

@dchvn Maybe could you rebase onto or merge master branch and see what happens? @zero323 Now that #34363 has been merged. Could you give it another pass?

Sure thing @ueshin. I'll review it after it is synced with master and  https://github.com/apache/spark/pull/34731#pullrequestreview-819670002 is addressed.

@dchvn Maybe could you rebase onto or merge master branch and see what happens? @zero323 Now that #34363 has been merged. Could you give it another pass?

Sure thing @ueshin. I'll review it after it is synced with master and #34731 (review) is addressed.

Updated, thanks!

Looks pretty good to me except https://github.com/apache/spark/pull/34731#pullrequestreview-819670002

Merged into master.
Thanks everyone!

Thanks all! ❤

authors: AmplabJenkins HyukjinKwon SparkQA dchvn itholic ueshin zero323


text:
[SPARK-39385][SQL] Supports push down REGR_AVGX and REGR_AVGY

What changes were proposed in this pull request?
https://github.com/apache/spark/pull/36773 translate linear regression aggregate functions for pushdown.
Although REGR_AVGX and REGR_AVGY are replaced to AVG in runtime, we can pushdown AVG to achieve the same result that push down REGR_AVGX and REGR_AVGY.
Why are the changes needed?
Let Aggregate pushdown supports REGR_AVGX and REGR_AVGY.
Does this PR introduce any user-facing change?
'No'.
New feature.
How was this patch tested?
New test cases.

ping @huaxingao cc @cloud-fan

So this PR is mostly for adding tests?

So this PR is mostly for adding tests?

Also fix the issue that JDBC dialect cannot compile REGR_AVGX and REGR_AVGY(In fact, is null literal in case when).

Can we clarify these in the PR description?

Can we clarify these in the PR description?

OK

thanks, merging to master!

@cloud-fan Thank you!

authors: beliefer cloud-fan


text:
[SPARK-39917][SQL] Use different error classes for numeric/interval arithmetic overflow

What changes were proposed in this pull request?

Similar with https://github.com/apache/spark/pull/37313, currently, when  arithmetic overflow errors happen under ANSI mode, the error messages are like
[ARITHMETIC_OVERFLOW] long overflow. Use 'try_multiply' to tolerate overflow and return NULL instead. If necessary set spark.sql.ansi.enabled to "false"

The "(except for ANSI interval type)" part is confusing. We should remove it for the numeric arithmetic operations and have a new error class for the interval division error: INTERVAL_ARITHMETIC_OVERFLOW
Why are the changes needed?

For better error messages
Does this PR introduce any user-facing change?

Yes, Use different error classes for arithmetic overflows of numeric/interval.. After changes, the error messages are simpler and more clear.
How was this patch tested?

UT

@MaxGekk @srielau I am moving forward to merge this error message improvement. We can discuss about the further improvement later.
@cloud-fan thanks for the review

The master branch failed to compile after merging this PR: https://github.com/apache/spark/runs/7616787860?check_suite_focus=true
I am reverting it to unblock the development of other contributors.

Thank you for swift recovering master branch, @gengliangwang .

authors: dongjoon-hyun gengliangwang


text:
[SPARK-39952][SQL] SaveIntoDataSourceCommand should recache result relation

What changes were proposed in this pull request?

recacheByPlan the result relation inside SaveIntoDataSourceCommand
Why are the changes needed?

The behavior of SaveIntoDataSourceCommand is similar with InsertIntoDataSourceCommand which supports append or overwirte data. In order to keep data consistent,  we should always do recacheByPlan the relation on post hoc.
Does this PR introduce any user-facing change?

yes, bug fix
How was this patch tested?

add test

the failed R test is irrelevant

thanks, merging to master/3.3/3.2/3.1!

authors: cloud-fan ulysses-you


text:
[SPARK-40713][CONNECT] Improve SET operation support in the proto and the server

What changes were proposed in this pull request?

Rename Union to SetOperation and make it cover except, intersect, union.
Improve server side support for SetOperation.

Why are the changes needed?

Improve SET operation support in the proto and the server.
Does this PR introduce any user-facing change?

No
How was this patch tested?

UT

Can one of the admins verify this patch?

please fix conflicts

thanks, merging to master!

authors: AmplabJenkins amaliujia cloud-fan


text:
[SPARK-41103][CONNECT][DOC] Document how to add a new proto field of messages

What changes were proposed in this pull request?

This PR adds a developer facing documentation for how to add proto messages to Connect proto. More specifically, adding how to add a proto field which takes considerations of required, optional and default values.
Why are the changes needed?

Improve documentation for developers who want to update Connect proto.
Does this PR introduce any user-facing change?

NO
How was this patch tested?

N/A

@grundprinzip @cloud-fan @HyukjinKwon @zhengruifeng @hvanhovell

From what I get we can explicitly set fields as optional and we get an hasXXX method

@grundprinzip ah let me check that. If so probably we need refactor proto  a lot.

TBF I only recently learned about this. If we can use it the refactoring will be worth it.

@grundprinzip
I am gonna to refactor the proto based on this then.

@zhengruifeng @HyukjinKwon @cloud-fan please take a look on this updated version of the proto style guide.

@grundprinzip suggestions applied. The doc look much better now with some more details filled in. Minding take another look?

thanks, merging to master!

Can one of the admins verify this patch?

authors: AmplabJenkins amaliujia cloud-fan grundprinzip


text:
[SPARK-47280][SQL] Remove timezone limitation for ORACLE TIMESTAMP WITH TIMEZONE

What changes were proposed in this pull request?

As illustrated by the Oracle Documentation:

TIMESTAMP WITH TIME ZONE and TIMESTAMP WITH LOCAL TIME ZONE types can be represented
as standard java.sql.Timestamp type.
The byte representation of TIMESTAMP WITH TIME ZONE and TIMESTAMP WITH LOCAL TIME ZONE
types to java.sql.Timestamp is straight forward.
This is because the internal format of TIMESTAMP WITH TIME ZONE and TIMESTAMP WITH LOCAL
TIME ZONE data types is GMT, and java.sql.Timestamp type objects internally use a
milliseconds time value that is the number of milliseconds since EPOCH.

As we use rs.getTimestamp instead of rs.getString or rs.getTIMESTAMPTZ to retrieve the timestamp value, it's safe to remove the spark.sql.session.timeZone guard when mapping catalyst type to oracle/jdbc types.
Why are the changes needed?

Improve the functionality of the Oracle connector
Does this PR introduce any user-facing change?

When reading Oracle TIMESTAMP WITH TIMEZONE, the spark.sql.session.timeZone can be changed without restriction for equivalence of JVM default.
How was this patch tested?

new unit tests added
Was this patch authored or co-authored using generative AI tooling?
no

cc @cloud-fan @dongjoon-hyun, thanks

Thanks for the review @cloud-fan.
Merged to master

Thank you @dongjoon-hyun


text:
[SPARK-48239][INFRA][FOLLOWUP] install the missing jq library

What changes were proposed in this pull request?

This is a followup of https://github.com/apache/spark/pull/46534 . We missed the jq library which is needed to create git tags.
Why are the changes needed?

fix bug
Does this PR introduce any user-facing change?

no
How was this patch tested?

manual
Was this patch authored or co-authored using generative AI tooling?

no

cc @HyukjinKwon @dongjoon-hyun

thanks for review, merging to master!


text:
[SPARK-8028] [SPARKR] Use addJar instead of setJars in SparkR

This prevents the spark.jars from being cleared while using --packages or --jars
cc @pwendell @davies @brkyvz

Merged build triggered.

Merged build started.

Test build #33946 has started for   PR 6568 at commit 3a9cf1f.

Yeah this makes sense.

I just tried this out on EC2 with something like ./bin/sparkR --packages com.databricks:spark-csv_2.10:1.0.3 and it works great now

LGTM. By the way, just noticed: RRDD could have been R2D2 :)

Test build #33946 has finished for   PR 6568 at commit 3a9cf1f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

authors: AmplabJenkins SparkQA brkyvz pwendell shivaram


text:
[SPARK-8072] [SQL] Better AnalysisException for writing DataFrame with identically named columns

Instead of adding rule to CheckAnalysis.scala, add the rule as a check in DataFrameWriter.scala.
Tests also included in DataFrameSuite.scala.

cc @rxin @marmbrus

NAVER - http://www.naver.com/
sujkh@naver.com 님께 보내신 메일 <[spark] [SPARK-8072] [SQL] Better AnalysisException for writing DataFrame with identically named columns (#7013)> 이 다음과 같은 이유로 전송 실패했습니다.

받는 사람이 회원님의 메일을 수신차단 하였습니다.

Can one of the admins verify this patch?

NAVER - http://www.naver.com/
sujkh@naver.com 님께 보내신 메일 <Re: [spark] [SPARK-8072] [SQL] Better AnalysisException for writing DataFrame with identically named columns (#7013)> 이 다음과 같은 이유로 전송 실패했습니다.

받는 사람이 회원님의 메일을 수신차단 하였습니다.

ok to test

Merged build triggered.

Merged build started.

LGTM

Test build #35826 has started for   PR 7013 at commit 7c3d928.

Test build #35826 has finished for   PR 7013 at commit 7c3d928.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

After talking more off-line with @rxin I think we want to make this check specific to parquet.  For other data sources (like CSV) its actually not a problem to have duplicate column names.

cc @rxin  @marmbrus
Can you review this?

Merged build triggered.

Merged build started.

Test build #36131 has started for   PR 7013 at commit 98b4399.

Test build #36131 has finished for   PR 7013 at commit 98b4399.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Merged build triggered.

Merged build started.

Test build #36136 has started for   PR 7013 at commit 3cc4d2c.

Test build #36136 has finished for   PR 7013 at commit 3cc4d2c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

@animeshbaranawal Do any other data sources also have this problem? I'm thinking of Orc and JSON, will JSON overwrite duplicated column silently?

Yes. I tried with JSON and it overwrites the data. Michael Armbrust also said that he want the rule for parquet only.

Let me clarify:  I want the error on a per datasource basis, contingent upon whether it makes sense given the limitations of the format.

@marmbrus Didn't get you? Am I missing something?

We should also do it for JSON.

And we should throw the error inside of parquet if possible.  That way we don't have tons of special case code inside of the generic data source handler.

Ideally, this would serve as an example so that other data source implementors could throw errors when people try to write out invalid data (i.e. consider a datasource that only allows alpha numeric characters in its column names).

Got it ! What about jdbc ?

Merged build triggered.

Merged build started.

Test build #36257 has started for   PR 7013 at commit a8a964f.

Test build #36257 has finished for   PR 7013 at commit a8a964f.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Merged build triggered.

Merged build started.

Test build #36263 has started for   PR 7013 at commit fd45e1b.

Test build #36263 has finished for   PR 7013 at commit fd45e1b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Build triggered.

Build started.

Test build #36345 has started for   PR 7013 at commit f70dd0e.

Test build #36345 has finished for   PR 7013 at commit f70dd0e.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds no public classes.

Build finished. Test PASSed.

Why is it not merging cleanly?

[marmbrus@Michaels-MacBook-Pro-2 spark ((14e4bf8...))]$ git checkout pr/7013
Previous HEAD position was 14e4bf8... Use CanBroadcast in broadcast outer join planning
Branch pr/7013 set up to track remote branch pr/7013 from origin.
Switched to a new branch 'pr/7013'

[marmbrus@Michaels-MacBook-Pro-2 spark (pr/7013)]$ git merge origin/master
Auto-merging sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala
Removing sql/hive/src/test/resources/golden/Date comparison test 2-0-dc1b267f1d79d49e6675afe4fd2a34a5
Removing sql/hive/src/test/resources/golden/Date cast-0-a7cd69b80c77a771a2c955db666be53d
Auto-merging sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUDFs.scala
Auto-merging sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
CONFLICT (content): Merge conflict in sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
Auto-merging sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala
Auto-merging sql/core/src/main/scala/org/apache/spark/sql/execution/pythonUDFs.scala
Auto-merging sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ScalaUDF.scala
Removing sql/catalyst/src/main/java/org/apache/spark/sql/BaseRow.java
Removing sql/catalyst/src/main/java/org/apache/spark/sql/BaseMutableRow.java
Automatic merge failed; fix conflicts and then commit the result.

Someone else has added tests to DataFrameSuite.

Fixed the conflict manually.  Merged to master.  Thanks!

@animeshbaranawal I think you want to add the email address you used in your commit to your github profile, so the commit will show up properly as yours.

added !

authors: AmplabJenkins SparkQA animeshbaranawal davies marmbrus rxin sujkh85