Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16461
Shouldn't we also remove the actual flag? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16461#discussion_r94421552
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala
---
@@ -436,7 +436,6 @@ abstract class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16371
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r94234184
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/RewriteDistinctAggregatesSuite.scala
---
@@ -63,7 +63,7 @@ class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r94233881
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/RewriteDistinctAggregatesSuite.scala
---
@@ -63,7 +63,7 @@ class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16233#discussion_r94222724
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -50,6 +50,36 @@ object SimpleAnalyzer extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16233#discussion_r94222500
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -50,6 +50,36 @@ object SimpleAnalyzer extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16417#discussion_r94187427
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
---
@@ -310,6 +310,31 @@ class
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16427
LGTM. Merging to master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16427#discussion_r94093538
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala ---
@@ -2585,9 +2586,12 @@ class Dataset[T] private[sql](
* Creates a local
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16233#discussion_r94041766
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -510,32 +510,121 @@ class Analyzer
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93714338
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala ---
@@ -146,18 +146,20 @@ class QueryExecution(val sparkSession
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93713654
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
---
@@ -78,11 +108,21 @@ case class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93712892
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
---
@@ -135,6 +167,15 @@ case class Cast(child: Expression
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93714063
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
---
@@ -78,11 +108,21 @@ case class
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93541632
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -104,6 +104,7 @@ class Analyzer
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16308#discussion_r93714774
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileFormatWriter.scala
---
@@ -288,7 +289,7 @@ object FileFormatWriter
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16233
@nsyca we actually support a 2-part name (look at TableIdentifier). You
don't have to define the database. We have the concept of a `currentDatabase`
in the `SessionCatalog`, and use th
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16371
@viirya I left a few comments on the PR. Lets wait for some consensus
before pushing ahead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93680880
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala
---
@@ -29,12 +30,10 @@ import
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93677896
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala
---
@@ -29,12 +30,10 @@ import
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93677654
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala
---
@@ -44,39 +43,52 @@ abstract class Collect
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93677313
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala
---
@@ -44,39 +43,52 @@ abstract class Collect
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93677262
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Percentile.scala
---
@@ -33,10 +33,9 @@ import
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16371#discussion_r93677104
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/collect.scala
---
@@ -44,39 +43,52 @@ abstract class Collect
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16381
LGTM. Merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16233
### Background
I think we should view this PR in the proper context. This PR is the first
PR in a series to get the following things done:
- Properly support nested views. The main
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16233#discussion_r93635804
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -510,32 +510,62 @@ class Analyzer(
* Replaces
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15980
LGTM. Merging to master/2.1/2.0. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16204
LGTM - merging to master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16204#discussion_r93535397
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -466,6 +466,19 @@ object SQLConf {
.longConf
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16204#discussion_r93528883
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -466,6 +466,19 @@ object SQLConf {
.longConf
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15980
I'll merge this after another test run. Ping me if you have objections.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15980
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16350
LGTM - merging to branch-2.0. Thanks!
Can you close this one? Our tooling only closes PRs against master.
---
If your project is set up for it, you can reply to this email and have your
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16350
Maybe we should just drop the UT (so we don't have to add the metrics). cc
@ericl WDYT?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16340
Merging to master/2.1/2.0. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16340
LGTM - pending jenkins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16057
LGTM - merging to master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16135
Merging to master/2.1. @xuanyuanking can you open a backport for 2.0, if we
also need to merge this to that branche?
---
If your project is set up for it, you can reply to this email and have
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16337
On 1 & 2 - it might be nice if we can use the spark JDBC connector to run
the tests directly on a different system. That would make comparisons trivial.
On 3 - what is the current
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16057#discussion_r93058473
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Percentile.scala
---
@@ -98,7 +92,7 @@ case class Percentile
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16337
@nsyca @kevinyu98
### 1. Correctness
> how about running the same set of test cases against a second SQL system
and comparing the results?
Yeah, that works. I am just trying to m
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16135
I am merging this one after a successful test run. Ping me if you object.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16135
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16337
@kevinyu98 this looks like pretty solid work. A couple of questions:
- This is a **huge** PR. This will be hard to review. Is there a change
that we can split this up in more digestible
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16337
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16057#discussion_r92995705
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Percentile.scala
---
@@ -98,7 +92,7 @@ case class Percentile
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16274
Merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16057#discussion_r92440234
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
---
@@ -673,48 +673,54 @@ object TypeCoercion
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16246#discussion_r92421093
--- Diff: sql/core/src/test/resources/sql-tests/inputs/scalar-subquery.sql
---
@@ -0,0 +1,20 @@
+CREATE OR REPLACE TEMPORARY VIEW p AS VALUES (1, 1
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16274
Two nits, otherwise LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16274#discussion_r92413275
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/types/ArrayType.scala ---
@@ -78,10 +78,10 @@ case class ArrayType(elementType: DataType
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16252#discussion_r92401784
--- Diff:
core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala ---
@@ -694,7 +694,7 @@ private[storage] class PartiallyUnrolledIterator
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16252#discussion_r9239
--- Diff:
core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala ---
@@ -694,7 +694,7 @@ private[storage] class PartiallyUnrolledIterator
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16246
Ok, I am merging this to master/2.1/2.0. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16246
LGTM - pending jenkins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16255#discussion_r92293834
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -416,8 +418,8 @@ object ColumnPruning extends Rule
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16232#discussion_r92289637
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
---
@@ -96,13 +98,35 @@ public UnsafeKVExternalSorter
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16232#discussion_r92289384
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
---
@@ -96,13 +98,35 @@ public UnsafeKVExternalSorter
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16232#discussion_r92289268
--- Diff:
sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
---
@@ -96,13 +98,35 @@ public UnsafeKVExternalSorter
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16246#discussion_r92286561
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
---
@@ -136,24 +142,36 @@ trait CheckAnalysis extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16246#discussion_r92285487
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
---
@@ -124,6 +124,12 @@ trait CheckAnalysis extends
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16255#discussion_r92245640
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -200,6 +200,8 @@ object RemoveAliasOnlyProject
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16262
@gatorsmile yeah you are right about that. @jiangxb1987 ignore my last
request :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16262
@jiangxb1987 could you open a backport for branch 2.1?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16262
LGTM - merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16255#discussion_r92197200
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -200,6 +200,8 @@ object RemoveAliasOnlyProject
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16255#discussion_r92186722
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -416,8 +418,8 @@ object ColumnPruning extends Rule
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15717
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16246
I think we need to revisit the rule in CheckAnalysis, and make it `Alias`
aware. We only need to keep track of the inner references. We traverse/recurse
down the tree and do a few things
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16255
@windpiger I am not sure that this is an analysis problem. I suspect that
optimizer is messing up the query. Could you pinpoint the rule that causes the
issue? Tip: in the spark shell (`./build
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16122
LGTM, merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16122
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16122
I am going to merge this after a successful build. Ping me if anyone
objects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16245#discussion_r91845297
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala
---
@@ -514,6 +514,19 @@ case class OptimizeCodegen(conf
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16232
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
Merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
It might at some point be an idea to add a bunch of asserts to `Platform`,
that would make diagnoses a lot easier.
---
If your project is set up for it, you can reply to this email and have
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
Ok, lets not do that for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
also cc @davies
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16235
LGTM - pending jenkins.
@kiszk do you think there is a sane way of testing this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16197
Merging to 2.0. Thanks! Can you close?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16197
LGTM - pending jenkins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16177
@aray could you open a backport for branch 2.0 if you feel that we should
also fix it there?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16177
LGTM. Merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15722
LGTM. Merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15722
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell closed the pull request at:
https://github.com/apache/spark/pull/16174
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16140
LGTM - merging to master/2.1. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
GitHub user hvanhovell opened a pull request:
https://github.com/apache/spark/pull/16174
[SPARK-18741][STREAMING] Reuse or clean-up SparkContext in streaming tests
## What changes were proposed in this pull request?
Tests in Spark Streaming currently create a `SparkContext` for
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16174
cc @tdas @zsxwing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/15722
@jiexiong PR descriptions are used in git commit messages, and should be
clear and concise. The fix LGTM, but the description should be improved for
future reference. How about we change it into
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/16170
Thanks for the review. Merging this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16168#discussion_r91054484
--- Diff:
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -114,9 +117,26 @@ private[hive] class HiveMetastoreCatalog
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16168#discussion_r91051441
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -207,31 +205,56 @@ case class CreateViewCommand
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16168#discussion_r91053965
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -509,32 +509,42 @@ class Analyzer(
* Replaces
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16168#discussion_r91053897
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -509,32 +509,42 @@ class Analyzer(
* Replaces
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/16168#discussion_r91053671
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
---
@@ -509,32 +509,42 @@ class Analyzer(
* Replaces
1401 - 1500 of 4178 matches
Mail list logo
