You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "zhuangchong (via GitHub)" <gi...@apache.org> on 2023/07/12 06:20:25 UTC

[GitHub] [incubator-paimon] zhuangchong opened a new pull request, #1547: [common] Fix can not create table by java api with ROW data type

zhuangchong opened a new pull request, #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
   Linked issue: close #1530 
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] FangYongs commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1632380540

   I think the root cause is because when a nested type such as row type/map/list is added with java api, the column id starts from 0. We should update the id in `Schema.Builder.column()` for nested type instead of `RowType.collectFieldIds`


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#discussion_r1264874639


##########
paimon-common/src/main/java/org/apache/paimon/types/RowType.java:
##########
@@ -170,7 +170,6 @@ public void collectFieldIds(Set<Integer> fieldIds) {
                         String.format("Broken schema, field id %s is duplicated.", field.id()));
             }
             fieldIds.add(field.id());
-            field.type().collectFieldIds(fieldIds);

Review Comment:
   Can we keep here as it is?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhuangchong commented on a diff in pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on code in PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#discussion_r1265007810


##########
paimon-core/src/main/java/org/apache/paimon/schema/Schema.java:
##########
@@ -243,6 +243,10 @@ public Builder column(String columnName, DataType dataType, @Nullable String des
             Preconditions.checkNotNull(columnName, "Column name must not be null.");
             Preconditions.checkNotNull(dataType, "Data type must not be null.");
             columns.add(new DataField(++highestFieldId, columnName, dataType, description));
+
+            Set<Integer> structuredFieldIds = new HashSet<>();
+            dataType.collectFieldIds(structuredFieldIds);
+            highestFieldId += structuredFieldIds.size();

Review Comment:
   I understand it is needed and I am modifying the `RowType.collectFieldIds` method.
   
   



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhuangchong commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1632075824

   > 
   
   I understand that modifying Schema.Builder cannot solve the problem that the field type is a nested structure of Row<Row<...>>, and the RowType.collectFieldIds method will still report the duplicated field id.
   
   ![image](https://github.com/apache/incubator-paimon/assets/37063904/93a3c09e-1a6f-42fc-8084-ca11fbdb4687)
   
   
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhuangchong commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1639919847

   > Perhaps we should discuss clearly why not just modify `Schema.Builder`.
   
   Because the Map type contains key type and value type, I cannot get all nested subtype fields in Schema.Builder to reset ID processing, I added the copy id method.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] JingsongLi commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1632421371

   > I think the root cause is because when a nested type such as row type/map/list is added with java api, the column id starts from 0. We should update the id in `Schema.Builder.column()` for nested type instead of `RowType.collectFieldIds`
   
   Yes, we can just update ids if there are duplicates.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] JingsongLi merged pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhuangchong commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1641401739

   > You need to take a look to `ReassignFieldId`. Actually, the final modification just be: In `Schema.Builder.column`, just use `ReassignFieldId` to create a new DataType.
   
   Thank you very much @JingsongLi .


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#discussion_r1266252094


##########
paimon-core/src/main/java/org/apache/paimon/schema/Schema.java:
##########
@@ -243,6 +243,10 @@ public Builder column(String columnName, DataType dataType, @Nullable String des
             Preconditions.checkNotNull(columnName, "Column name must not be null.");
             Preconditions.checkNotNull(dataType, "Data type must not be null.");
             columns.add(new DataField(++highestFieldId, columnName, dataType, description));
+
+            Set<Integer> structuredFieldIds = new HashSet<>();
+            dataType.collectFieldIds(structuredFieldIds);
+            highestFieldId += structuredFieldIds.size();

Review Comment:
   Why?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] JingsongLi commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1631978417

   Field Id should be unique. It is correct.
   
   Maybe we can enhance `Schema.Builder` to nested Row field.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhuangchong closed pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong closed pull request #1547: [common] Fix can not create table by java api with ROW data type
URL: https://github.com/apache/incubator-paimon/pull/1547


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] FangYongs commented on a diff in pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "FangYongs (via GitHub)" <gi...@apache.org>.
FangYongs commented on code in PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#discussion_r1264910152


##########
paimon-core/src/main/java/org/apache/paimon/schema/Schema.java:
##########
@@ -243,6 +243,10 @@ public Builder column(String columnName, DataType dataType, @Nullable String des
             Preconditions.checkNotNull(columnName, "Column name must not be null.");
             Preconditions.checkNotNull(dataType, "Data type must not be null.");
             columns.add(new DataField(++highestFieldId, columnName, dataType, description));
+
+            Set<Integer> structuredFieldIds = new HashSet<>();
+            dataType.collectFieldIds(structuredFieldIds);
+            highestFieldId += structuredFieldIds.size();

Review Comment:
   Should we need to update the id in the nested data type?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-paimon] zhangjun0x01 commented on pull request #1547: [common] Fix can not create table by java api with ROW data type

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on PR #1547:
URL: https://github.com/apache/incubator-paimon/pull/1547#issuecomment-1635231724

   maybe we can have users specify IDs, refer to iceberg : https://iceberg.apache.org/docs/latest/java-api-quickstart/#create-a-schema 


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org