[spark] Replace create existing tag semantic with replace_tag #4346

askwang · 2024-10-18T01:58:10Z

Purpose

In default way, the create_tag procedure for an existing tag means to update it or fail, it's semantic is not very accurate, replace it with repalce_tag procedure.

before semantic

CALL paimon.sys.create_tag(table => 'test.T', tag => 'test_tag', snapshot => 1)")

-- success. update it
CALL paimon.sys.create_tag(table => 'test.T', tag => 'test_tag', snapshot => 1, time_retained => '1 d')")

-- failed
CALL paimon.sys.create_tag(table => 'test.T', tag => 'test_tag', snapshot => 2)")

after semantic

CALL paimon.sys.create_tag(table => 'test.T', tag => 'test_tag', snapshot => 1)")

-- failed
CALL paimon.sys.create_tag(table => 'test.T', tag => 'test_tag', snapshot => 1, time_retained => '1 d')")

-- success. use replace to update
CALL paimon.sys.replace_tag(table => 'test.T', tag => 'test_tag', snapshot => 2, time_retained => '1 d')")

Tests

API and Format

Documentation

wwj6591812

1、I think update_tag may be better?
2、You alse need modify flink action&& procedure and doc.

wwj6591812 · 2024-10-19T00:04:54Z

docs/content/spark/procedures.md

@@ -121,6 +121,19 @@ This section introduce all available spark procedures about paimon.
         CALL sys.rename_tag(table => 'default.T', tag_name => 'tag1', target_tag_name => 'tag2')
      </td>
    </tr>
+    <tr>
+      <td>replace_tag</td>


I think update_tag may be better?

wwj6591812 · 2024-10-19T00:06:51Z

docs/content/spark/procedures.md

+         Replace an existing tag with new tag info. Arguments:
+            <li>table: the target table identifier. Cannot be empty.</li>
+            <li>tag: name of the existed tag. Cannot be empty.</li>
+            <li>snapshot(Long):  id of the snapshot which the tag is based on.</li>


1、snapshotId
2、It is optional.

wwj6591812 · 2024-10-19T00:10:29Z

docs/content/spark/procedures.md

+            <li>table: the target table identifier. Cannot be empty.</li>
+            <li>tag: name of the existed tag. Cannot be empty.</li>
+            <li>snapshot(Long):  id of the snapshot which the tag is based on.</li>
+            <li>time_retained: The maximum time retained for the existing tag.</li>


2、It is optional.

wwj6591812 · 2024-10-19T00:12:46Z

paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java

+    public void replaceTag(String tagName, @Nullable Duration timeRetained) {
+        Snapshot latestSnapshot = snapshotManager().latestSnapshot();
+        SnapshotNotExistException.checkNotNull(
+                latestSnapshot, "Cannot create tag because latest snapshot doesn't exist.");


Can not update tag because latest snapshot doesn't exist.

wwj6591812 · 2024-10-19T00:19:18Z

paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java

+                        findSnapshot(fromSnapshotId),
+                        tagName,
+                        timeRetained,
+                        store().createTagCallbacks());


Here you should think this case:
If user update a tag, but he don't want call createTagCallbacks.

good, replace the existing tag is no need to call callback again.

wwj6591812 · 2024-10-19T00:25:11Z

paimon-core/src/main/java/org/apache/paimon/table/Table.java

@@ -120,6 +120,12 @@ default String fullName() {
    @Experimental
    void renameTag(String tagName, String targetTagName);

+    @Experimental
+    void replaceTag(String tagName, Duration timeRetained);


接口方法【replaceTag】必须使用javadoc注释

wwj6591812 · 2024-10-19T00:25:11Z

paimon-core/src/main/java/org/apache/paimon/utils/TagManager.java


+    public void createOrReplaceTag(


createOrUpdateTag

wwj6591812 · 2024-10-19T00:25:41Z

.../paimon-spark-common/src/main/java/org/apache/paimon/spark/procedure/CreateTagProcedure.java

-                    new StructField[] {
-                        new StructField("result", DataTypes.BooleanType, true, Metadata.empty())
-                    });
+public class CreateTagProcedure extends CreateOrReplaceTagBaseProcedure {

    protected CreateTagProcedure(TableCatalog tableCatalog) {


why not private?

wwj6591812 · 2024-10-19T00:26:09Z

...mon-spark-common/src/main/java/org/apache/paimon/spark/procedure/ReplaceTagTagProcedure.java

+import java.time.Duration;
+
+/** A procedure to replace a tag. */
+public class ReplaceTagTagProcedure extends CreateOrReplaceTagBaseProcedure {


UpdateTagProcedure

wwj6591812 · 2024-10-19T00:26:09Z

...mon-spark-common/src/main/java/org/apache/paimon/spark/procedure/ReplaceTagTagProcedure.java

+/** A procedure to replace a tag. */
+public class ReplaceTagTagProcedure extends CreateOrReplaceTagBaseProcedure {
+
+    protected ReplaceTagTagProcedure(TableCatalog tableCatalog) {


why not private?

askwang · 2024-10-21T09:30:02Z

addressed comments, thanks for review @wwj6591812
A little difference, keep the name 'replace_tag' not change. I think it's name is ok, and iceberg also uses 'replace tag' to update tag. what do you think?

wwj6591812 · 2024-10-24T12:01:01Z

addressed comments, thanks for review @wwj6591812 A little difference, keep the name 'replace_tag' not change. I think it's name is ok, and iceberg also uses 'replace tag' to update tag. what do you think?

ok

JingsongLi · 2024-10-29T07:50:53Z

paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java

+    }
+
+    @Override
+    public void replaceTag(String tagName, long fromSnapshotId, @Nullable Duration timeRetained) {


Just create a replaceTag(String tagName, @Nullable Long fromSnapshotId, @Nullable Duration timeRetained)?

JingsongLi

+1

…#4346)

askwang added 2 commits October 17, 2024 23:44

replace-tag-procedure

d7423fa

fix test

e8949e1

wwj6591812 reviewed Oct 19, 2024

View reviewed changes

fix comments and add flink

ddee00b

fix test

084c5fd

JingsongLi reviewed Oct 29, 2024

View reviewed changes

remove one method

3caa5de

JingsongLi approved these changes Oct 31, 2024

View reviewed changes

JingsongLi merged commit e2acdc2 into apache:master Oct 31, 2024
13 checks passed

askwang mentioned this pull request Nov 5, 2024

[spark] Add replace tag ddl syntax #4457

Merged

hang8929201 pushed a commit to hang8929201/paimon that referenced this pull request Nov 7, 2024

[spark] Replace create existing tag semantic with replace_tag (apache…

2b31960

…#4346)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[spark] Replace create existing tag semantic with replace_tag #4346

[spark] Replace create existing tag semantic with replace_tag #4346

askwang commented Oct 18, 2024 •

edited

Loading

wwj6591812 left a comment

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

askwang Oct 21, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

wwj6591812 Oct 19, 2024

askwang commented Oct 21, 2024

wwj6591812 commented Oct 24, 2024

JingsongLi Oct 29, 2024

askwang Oct 30, 2024

JingsongLi left a comment

[spark] Replace create existing tag semantic with replace_tag #4346

[spark] Replace create existing tag semantic with replace_tag #4346

Conversation

askwang commented Oct 18, 2024 • edited Loading

Purpose

Tests

API and Format

Documentation

wwj6591812 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

askwang commented Oct 21, 2024

wwj6591812 commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JingsongLi left a comment

Choose a reason for hiding this comment

askwang commented Oct 18, 2024 •

edited

Loading