Skip to content

Commit be2f1b8

Browse files
committed
address comments
1 parent f6f031f commit be2f1b8

File tree

4 files changed

+15
-14
lines changed

4 files changed

+15
-14
lines changed

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ intervalUnit
846846
;
847847

848848
colPosition
849-
: position=FIRST | position=AFTER multipartIdentifier
849+
: position=FIRST | position=AFTER afterCol=errorCapturingIdentifier
850850
;
851851

852852
dataType

sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/TableChange.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ public int hashCode() {
301301
interface ColumnPosition {
302302
First FIRST = new First();
303303

304-
static ColumnPosition After(String[] column) {
304+
static ColumnPosition createAfter(String column) {
305305
return new After(column);
306306
}
307307
}
@@ -320,35 +320,37 @@ public String toString() {
320320

321321
/**
322322
* Column position AFTER means the specified column should be put after the given `column`.
323+
* Note that, the specified column may be a nested field, and then the given `column` refers to
324+
* a nested field in the same struct.
323325
*/
324326
final class After implements ColumnPosition {
325-
private final String[] column;
327+
private final String column;
326328

327-
private After(String[] column) {
329+
private After(String column) {
328330
assert column != null;
329331
this.column = column;
330332
}
331333

332-
public String[] getColumn() {
334+
public String getColumn() {
333335
return column;
334336
}
335337

336338
@Override
337339
public String toString() {
338-
return "AFTER " + CatalogV2Implicits.quoteNameParts(column);
340+
return "AFTER " + column;
339341
}
340342

341343
@Override
342344
public boolean equals(Object o) {
343345
if (this == o) return true;
344346
if (o == null || getClass() != o.getClass()) return false;
345347
After after = (After) o;
346-
return Arrays.equals(column, after.column);
348+
return column.equals(after.column);
347349
}
348350

349351
@Override
350352
public int hashCode() {
351-
return Arrays.hashCode(column);
353+
return Objects.hash(column);
352354
}
353355
}
354356

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2807,7 +2807,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
28072807
ctx.position.getType match {
28082808
case SqlBaseParser.FIRST => ColumnPosition.FIRST
28092809
case SqlBaseParser.AFTER =>
2810-
ColumnPosition.After(typedVisit[Seq[String]](ctx.multipartIdentifier).toArray)
2810+
ColumnPosition.createAfter(ctx.afterCol.getText)
28112811
}
28122812
}
28132813

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, GlobalTempView, Loc
2424
import org.apache.spark.sql.catalyst.catalog.BucketSpec
2525
import org.apache.spark.sql.catalyst.expressions.{EqualTo, Literal}
2626
import org.apache.spark.sql.catalyst.plans.logical._
27-
import org.apache.spark.sql.connector.catalog.TableChange.ColumnPosition.After
28-
import org.apache.spark.sql.connector.catalog.TableChange.ColumnPosition.FIRST
27+
import org.apache.spark.sql.connector.catalog.TableChange.ColumnPosition.{createAfter, FIRST}
2928
import org.apache.spark.sql.connector.expressions.{ApplyTransform, BucketTransform, DaysTransform, FieldReference, HoursTransform, IdentityTransform, LiteralValue, MonthsTransform, Transform, YearsTransform}
3029
import org.apache.spark.sql.types.{IntegerType, LongType, StringType, StructType, TimestampType}
3130
import org.apache.spark.unsafe.types.UTF8String
@@ -528,7 +527,7 @@ class DDLParserSuite extends AnalysisTest {
528527
comparePlans(
529528
parsePlan("ALTER TABLE table_name ADD COLUMN x int AFTER y"),
530529
AlterTableAddColumnsStatement(Seq("table_name"), Seq(
531-
QualifiedColType(Seq("x"), IntegerType, None, Some(After(Array("y"))))
530+
QualifiedColType(Seq("x"), IntegerType, None, Some(createAfter("y")))
532531
)))
533532
}
534533

@@ -618,13 +617,13 @@ class DDLParserSuite extends AnalysisTest {
618617
test("alter table: update column type, comment and position") {
619618
comparePlans(
620619
parsePlan("ALTER TABLE table_name CHANGE COLUMN a.b.c " +
621-
"TYPE bigint COMMENT 'new comment' AFTER x.y"),
620+
"TYPE bigint COMMENT 'new comment' AFTER d"),
622621
AlterTableAlterColumnStatement(
623622
Seq("table_name"),
624623
Seq("a", "b", "c"),
625624
Some(LongType),
626625
Some("new comment"),
627-
Some(After(Array("x", "y")))))
626+
Some(createAfter("d"))))
628627
}
629628

630629
test("alter table: drop column") {

0 commit comments

Comments
 (0)