LinMingQiang commented on code in PR #4947:
URL: https://github.com/apache/paimon/pull/4947#discussion_r1921966579
##########
docs/content/spark/procedures.md:
##########
@@ -159,13 +159,16 @@ This section introduce all available spark procedures
about paimon.
<tr>
<td>rollback</td>
<td>
- To rollback to a specific version of target table. Argument:
+ To rollback to a specific version of target table, note
version/snapshot/tag must set one of them. Argument:
<li>table: the target table identifier. Cannot be empty.</li>
<li>version: id of the snapshot or name of tag that will roll back
to.</li>
+ <li>snapshot: snapshot that will roll back to.</li>
+ <li>tag: tag that will roll back to.</li>
</td>
<td>
CALL sys.rollback(table => 'default.T', version =>
'my_tag')<br/><br/>
- CALL sys.rollback(table => 'default.T', version => 10)
+ CALL sys.rollback(table => 'default.T', version => 10)<br/><br/>
+ CALL sys.rollback(table => 'default.T', version => '20250122', type
=> 'tag')
</td>
Review Comment:
please fix this too
##########
docs/content/spark/procedures.md:
##########
@@ -159,13 +159,16 @@ This section introduce all available spark procedures
about paimon.
<tr>
<td>rollback</td>
<td>
- To rollback to a specific version of target table. Argument:
+ To rollback to a specific version of target table, note
version/snapshot/tag must set one of them. Argument:
<li>table: the target table identifier. Cannot be empty.</li>
<li>version: id of the snapshot or name of tag that will roll back
to.</li>
+ <li>snapshot: snapshot that will roll back to.</li>
Review Comment:
Mark vsersion is `Deprecated`.
##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/procedure/RollbackProcedure.java:
##########
@@ -61,15 +64,34 @@ public StructType outputType() {
@Override
public InternalRow[] call(InternalRow args) {
Identifier tableIdent = toIdentifier(args.getString(0),
PARAMETERS[0].name());
- String version = args.getString(1);
+ String version = args.isNullAt(1) ? null : args.getString(1);
+ Long snapshot = args.isNullAt(2) ? null : args.getLong(2);
+ String tag = args.isNullAt(3) ? null : args.getString(3);
+ if ((version != null && snapshot != null)
+ || (snapshot != null && tag != null)
+ || (tag != null && version != null)) {
+ throw new IllegalArgumentException(
+ "only can set one of version/snapshot/tag in
RollbackProcedure.");
+ }
+
+ if (version == null && snapshot == null && tag == null) {
+ throw new IllegalArgumentException(
+ "must set one of version/snapshot/tag in
RollbackProcedure.");
+ }
return modifyPaimonTable(
tableIdent,
table -> {
- if (version.chars().allMatch(Character::isDigit)) {
- table.rollbackTo(Long.parseLong(version));
+ if (snapshot != null) {
+ table.rollbackTo(snapshot);
+ } else if (tag != null) {
+ table.rollbackTo(tag);
} else {
- table.rollbackTo(version);
+ if (version.chars().allMatch(Character::isDigit)) {
+ table.rollbackTo(Long.parseLong(version));
+ } else {
+ table.rollbackTo(version);
+ }
Review Comment:
Based on the above modifications. snapshot != null ?
table.rollbackTo(snapshot) : table.rollbackTo(tag);
##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/procedure/RollbackProcedure.java:
##########
@@ -61,15 +64,34 @@ public StructType outputType() {
@Override
public InternalRow[] call(InternalRow args) {
Identifier tableIdent = toIdentifier(args.getString(0),
PARAMETERS[0].name());
- String version = args.getString(1);
+ String version = args.isNullAt(1) ? null : args.getString(1);
+ Long snapshot = args.isNullAt(2) ? null : args.getLong(2);
+ String tag = args.isNullAt(3) ? null : args.getString(3);
+ if ((version != null && snapshot != null)
+ || (snapshot != null && tag != null)
+ || (tag != null && version != null)) {
+ throw new IllegalArgumentException(
+ "only can set one of version/snapshot/tag in
RollbackProcedure.");
+ }
+
+ if (version == null && snapshot == null && tag == null) {
+ throw new IllegalArgumentException(
+ "must set one of version/snapshot/tag in
RollbackProcedure.");
+ }
Review Comment:
Use `Preconditions.check` in the beginning.
##########
paimon-spark/paimon-spark-common/src/main/java/org/apache/paimon/spark/procedure/RollbackProcedure.java:
##########
@@ -61,15 +64,34 @@ public StructType outputType() {
@Override
public InternalRow[] call(InternalRow args) {
Identifier tableIdent = toIdentifier(args.getString(0),
PARAMETERS[0].name());
- String version = args.getString(1);
+ String version = args.isNullAt(1) ? null : args.getString(1);
+ Long snapshot = args.isNullAt(2) ? null : args.getLong(2);
+ String tag = args.isNullAt(3) ? null : args.getString(3);
+ if ((version != null && snapshot != null)
+ || (snapshot != null && tag != null)
+ || (tag != null && version != null)) {
+ throw new IllegalArgumentException(
+ "only can set one of version/snapshot/tag in
RollbackProcedure.");
+ }
Review Comment:
if (version != null) {
1 : parse version
2: Assign to snapshot or tagName.
} else {
Preconditions.check -> `snapshot` and `tagName` cannot be set at the same
time.
}
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
