Skip to content

Commit 8a1bc89

Browse files
author
fangbo
committed
fix cr issues
1 parent e2c5af5 commit 8a1bc89

7 files changed

Lines changed: 103 additions & 12 deletions

File tree

‎lance-spark-3.4_2.12/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/LanceSqlExtensionsAstBuilder.scala‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import org.antlr.v4.runtime.ParserRuleContext
1717
import org.apache.spark.sql.catalyst.analysis.{UnresolvedIdentifier, UnresolvedRelation}
1818
import org.apache.spark.sql.catalyst.parser.{ParseException, ParserInterface}
1919
import org.apache.spark.sql.catalyst.plans.logical.{AddColumnsBackfill, AddIndex, LanceCreateBranch, LanceDropBranch, LanceDropIndex, LanceNamedArgument, LanceShowBranches, LogicalPlan, Optimize, SetUnenforcedPrimaryKey, ShowIndexes, UpdateColumnsBackfill, Vacuum}
20+
import org.apache.spark.sql.errors.QueryParsingErrors
2021
import org.lance.spark.utils.ParserUtils
2122

2223
import scala.collection.JavaConverters._
@@ -140,7 +141,12 @@ class LanceSqlExtensionsAstBuilder(delegate: ParserInterface)
140141
java.lang.Long.valueOf(value)
141142
} catch {
142143
case _: NumberFormatException =>
143-
throw new ParseException("Can't parse value:" + value + " to version", ctx)
144+
throw new ParseException(
145+
errorClass = "INVALID_TYPED_LITERAL",
146+
messageParameters = Map(
147+
"valueType" -> "LONG",
148+
"value" -> value),
149+
ctx)
144150
}
145151
}
146152

‎lance-spark-3.5_2.12/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/LanceSqlExtensionsAstBuilder.scala‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,12 @@ class LanceSqlExtensionsAstBuilder(delegate: ParserInterface)
144144
java.lang.Long.valueOf(value)
145145
} catch {
146146
case _: NumberFormatException =>
147-
throw new ParseException("Can't parse value:" + value + " to version", ctx)
147+
throw new ParseException(
148+
errorClass = "INVALID_TYPED_LITERAL",
149+
messageParameters = Map(
150+
"valueType" -> "LONG",
151+
"value" -> value),
152+
ctx)
148153
}
149154
}
150155

‎lance-spark-4.0_2.13/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/LanceSqlExtensionsAstBuilder.scala‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,12 @@ class LanceSqlExtensionsAstBuilder(delegate: ParserInterface)
140140
java.lang.Long.valueOf(value)
141141
} catch {
142142
case _: NumberFormatException =>
143-
throw new ParseException("Can't parse value:" + value + " to version", ctx)
143+
throw new ParseException(
144+
errorClass = "INVALID_TYPED_LITERAL",
145+
messageParameters = Map(
146+
"valueType" -> "LONG",
147+
"value" -> value),
148+
ctx)
144149
}
145150
}
146151

‎lance-spark-4.1_2.13/src/main/scala/org/apache/spark/sql/catalyst/parser/extensions/LanceSqlExtensionsAstBuilder.scala‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@
1313
*/
1414
package org.apache.spark.sql.catalyst.parser.extensions
1515

16+
import org.antlr.v4.runtime.ParserRuleContext
1617
import org.apache.spark.sql.catalyst.analysis.{UnresolvedIdentifier, UnresolvedRelation}
17-
import org.apache.spark.sql.catalyst.parser.ParserInterface
18+
import org.apache.spark.sql.catalyst.parser.{ParseException, ParserInterface}
1819
import org.apache.spark.sql.catalyst.plans.logical.{AddColumnsBackfill, AddIndex, LanceCreateBranch, LanceDropBranch, LanceDropIndex, LanceNamedArgument, LanceShowBranches, LogicalPlan, Optimize, SetUnenforcedPrimaryKey, ShowIndexes, UpdateColumnsBackfill, Vacuum}
1920
import org.lance.spark.utils.ParserUtils
2021

@@ -115,7 +116,7 @@ class LanceSqlExtensionsAstBuilder(delegate: ParserInterface)
115116
else LanceCreateBranch(
116117
table,
117118
branchName,
118-
org.lance.Ref.ofMain(java.lang.Long.valueOf(ctx.refMainVersion.getText)),
119+
org.lance.Ref.ofMain(_parseVersion(ctx, ctx.refMainVersion.getText)),
119120
ifNotExists)
120121
}
121122

@@ -130,10 +131,24 @@ class LanceSqlExtensionsAstBuilder(delegate: ParserInterface)
130131
else LanceCreateBranch(
131132
table,
132133
branchName,
133-
org.lance.Ref.ofBranch(refBranchName, java.lang.Long.valueOf(ctx.refBranchVersion.getText)),
134+
org.lance.Ref.ofBranch(refBranchName, _parseVersion(ctx, ctx.refBranchVersion.getText)),
134135
ifNotExists)
135136
}
136137

138+
def _parseVersion(ctx: ParserRuleContext, value: String): Long = {
139+
try {
140+
java.lang.Long.valueOf(value)
141+
} catch {
142+
case _: NumberFormatException =>
143+
throw new ParseException(
144+
errorClass = "INVALID_TYPED_LITERAL",
145+
messageParameters = Map(
146+
"valueType" -> "LONG",
147+
"value" -> value),
148+
ctx)
149+
}
150+
}
151+
137152
override def visitCreateBranchRefTag(ctx: LanceSqlExtensionsParser.CreateBranchRefTagContext)
138153
: LanceCreateBranch = {
139154
val table = UnresolvedIdentifier(visitMultipartIdentifier(ctx.multipartIdentifier()))

‎lance-spark-base_2.12/src/main/antlr4/org/apache/spark/sql/catalyst/parser/extensions/LanceSqlExtensions.g4‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ IN: 'IN';
9393
INDEX: 'INDEX';
9494
INDEXES: 'INDEXES';
9595
KEY: 'KEY';
96-
MAIN: 'MAIN';
9796
NOT: 'NOT';
9897
OF: 'OF';
9998
OPTIMIZE: 'OPTIMIZE';

‎lance-spark-base_2.12/src/main/scala/org/apache/spark/sql/execution/datasources/v2/LanceCreateBranchExec.scala‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import org.apache.spark.sql.catalyst.expressions.{Attribute, GenericInternalRow}
1818
import org.apache.spark.sql.catalyst.plans.logical.LanceCreateBranchOutputType
1919
import org.apache.spark.sql.connector.catalog.{Identifier, TableCatalog}
2020
import org.apache.spark.unsafe.types.UTF8String
21-
import org.lance.spark.{BaseLanceNamespaceSparkCatalog, LanceDataset}
21+
import org.lance.spark.LanceDataset
2222
import org.lance.spark.utils.Utils
2323

2424
import scala.collection.JavaConverters._
@@ -47,7 +47,10 @@ case class LanceCreateBranchExec(
4747
if (!ifNotExists || !alreadyExists) {
4848
var branchDs: org.lance.Dataset = null
4949
try {
50-
branchDs = dataset.createBranch(branchName, ref, dataset.getInitialStorageOptions)
50+
val opts = dataset.getInitialStorageOptions
51+
branchDs =
52+
if (opts == null || opts.isEmpty) dataset.createBranch(branchName, ref)
53+
else dataset.createBranch(branchName, ref, opts)
5154
} finally {
5255
if (branchDs != null) {
5356
branchDs.close()

‎lance-spark-base_2.12/src/test/java/org/lance/spark/branch/BaseBranchDDLTest.java‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,66 @@ public void testCreateBranchFromSpecificTag() {
211211
Assertions.assertEquals(versions.firstInsertVersion, branch.parentVersion);
212212
}
213213

214+
@Test
215+
public void testCreateBranchFailsWhenSourceBranchDoesNotExist() {
216+
prepareDatasetWithHistory();
217+
218+
Assertions.assertThrows(
219+
Exception.class,
220+
() ->
221+
spark
222+
.sql(
223+
String.format(
224+
"alter table %s create branch branch_from_missing_branch "
225+
+ "as of branch missing_branch",
226+
fullTable))
227+
.collectAsList());
228+
229+
Map<String, BranchInfo> branches =
230+
showBranches(String.format("show branches from %s", fullTable));
231+
Assertions.assertFalse(branches.containsKey("branch_from_missing_branch"));
232+
}
233+
234+
@Test
235+
public void testCreateBranchFailsWhenSourceTagDoesNotExist() {
236+
prepareDatasetWithHistory();
237+
238+
Assertions.assertThrows(
239+
Exception.class,
240+
() ->
241+
spark
242+
.sql(
243+
String.format(
244+
"alter table %s create branch branch_from_missing_tag as of tag missing_tag",
245+
fullTable))
246+
.collectAsList());
247+
248+
Map<String, BranchInfo> branches =
249+
showBranches(String.format("show branches from %s", fullTable));
250+
Assertions.assertFalse(branches.containsKey("branch_from_missing_tag"));
251+
}
252+
253+
@Test
254+
public void testCreateBranchFailsWhenSourceVersionDoesNotExist() {
255+
DatasetVersions versions = prepareDatasetWithHistory();
256+
long missingVersion = versions.latestVersion + 1000L;
257+
258+
Assertions.assertThrows(
259+
Exception.class,
260+
() ->
261+
spark
262+
.sql(
263+
String.format(
264+
"alter table %s create branch branch_from_missing_version "
265+
+ "as of version %d",
266+
fullTable, missingVersion))
267+
.collectAsList());
268+
269+
Map<String, BranchInfo> branches =
270+
showBranches(String.format("show branches from %s", fullTable));
271+
Assertions.assertFalse(branches.containsKey("branch_from_missing_version"));
272+
}
273+
214274
@Test
215275
public void testCreateBranchIfNotExistsNoOpWhenBranchExists() {
216276
DatasetVersions versions = prepareDatasetWithHistory();
@@ -221,9 +281,7 @@ public void testCreateBranchIfNotExistsNoOpWhenBranchExists() {
221281

222282
Dataset<Row> result =
223283
spark.sql(
224-
String.format(
225-
"alter table %s create branch if not exists existing_branch as of version %d",
226-
fullTable, versions.firstInsertVersion));
284+
String.format("alter table %s create branch if not exists existing_branch", fullTable));
227285

228286
assertSingleNameSchema(result);
229287
Assertions.assertEquals("existing_branch", result.collectAsList().get(0).getString(0));

0 commit comments

Comments
 (0)