maropu commented on a change in pull request #32787:
URL: https://github.com/apache/spark/pull/32787#discussion_r649726942



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -115,14 +115,16 @@ object FakeV2SessionCatalog extends TableCatalog {
  *                              if `t` was a permanent table when the current 
view was created, it
  *                              should still be a permanent table when 
resolving the current view,
  *                              even if a temp view `t` has been created.
+ * @param outerPlans outer query plans.
  */
 case class AnalysisContext(
     catalogAndNamespace: Seq[String] = Nil,
     nestedViewDepth: Int = 0,
     maxNestedViewDepth: Int = -1,
     relationCache: mutable.Map[Seq[String], LogicalPlan] = mutable.Map.empty,
     referredTempViewNames: Seq[Seq[String]] = Seq.empty,
-    referredTempFunctionNames: Seq[String] = Seq.empty)
+    referredTempFunctionNames: Seq[String] = Seq.empty,
+    outerPlans: Seq[LogicalPlan] = Seq.empty)

Review comment:
       Could you update the comment above? 
https://github.com/apache/spark/pull/32787/files#diff-ed19f376a63eba52eea59ca71f3355d4495fad4fad4db9a3324aade0d4986a47R97-R99

##########
File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveSubquerySuite.scala
##########
@@ -177,4 +178,61 @@ class ResolveSubquerySuite extends AnalysisTest {
       condition = Some(sum('a) === sum('c)))
     assertAnalysisError(plan, Seq("Invalid expressions: [sum(a), sum(c)]"))
   }
+
+  test("SPARK-35618: lateral join with star expansion") {

Review comment:
       Could you add tests for case-sensitivities?

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -115,14 +115,16 @@ object FakeV2SessionCatalog extends TableCatalog {
  *                              if `t` was a permanent table when the current 
view was created, it
  *                              should still be a permanent table when 
resolving the current view,
  *                              even if a temp view `t` has been created.
+ * @param outerPlans outer query plans.

Review comment:
       nit: `outer` -> `Outer`. Also, could you describe more about what's this 
param used for? (just like the other params)

##########
File path: sql/core/src/test/resources/sql-tests/inputs/join-lateral.sql
##########
@@ -12,9 +12,8 @@ SELECT * FROM t1, LATERAL (SELECT t1.c1 + t2.c1 FROM t2);
 -- lateral join with star expansion
 SELECT * FROM t1, LATERAL (SELECT *);
 SELECT * FROM t1, LATERAL (SELECT * FROM t2);
--- TODO(SPARK-35618): resolve star expressions in subquery
--- SELECT * FROM t1, LATERAL (SELECT t1.*);
--- SELECT * FROM t1, LATERAL (SELECT t1.*, t2.* FROM t2);
+SELECT * FROM t1, LATERAL (SELECT t1.*);
+SELECT * FROM t1, LATERAL (SELECT t1.*, t2.* FROM t2);

Review comment:
       Could you add tests for more deeply-nested cases? e.g.,
   ```
   SELECT * FROM t1, LATERAL (SELECT t1.*, t2.* FROM t2, LATERAL (SELECT t1.*, 
t2.*, t3.* FROM t3));
   ```

##########
File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveSubquerySuite.scala
##########
@@ -177,4 +178,61 @@ class ResolveSubquerySuite extends AnalysisTest {
       condition = Some(sum('a) === sum('c)))
     assertAnalysisError(plan, Seq("Invalid expressions: [sum(a), sum(c)]"))
   }
+
+  test("SPARK-35618: lateral join with star expansion") {

Review comment:
       Also, I think we need some tests for 
`spark.sql.parser.quotedRegexColumnNames`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to