Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug report] create Iceberg table failed if comment is null #1546

Closed
FANNG1 opened this issue Jan 17, 2024 · 1 comment · Fixed by #1653
Closed

[Bug report] create Iceberg table failed if comment is null #1546

FANNG1 opened this issue Jan 17, 2024 · 1 comment · Fixed by #1653
Assignees
@FANNG1 @Clearvive
Milestone
Gravitino 0.4.0

Comments

@FANNG1
Copy link
Contributor

FANNG1 commented Jan 17, 2024

Describe what's wrong

create Iceberg table failed if comment is null

Error message and/or stacktrace

java.lang.IllegalArgumentException: Failed to operate table(s) [iceberg_it_table_57dc4f9e] operation [CREATE] under schema [iceberg_it_schema_60a9a4a2], reason [IllegalArgumentException]
java.lang.IllegalArgumentException: Invalid value for properties [comment]: null
	at org.apache.iceberg.relocated.com.google.common.base.Preconditions.checkArgument(Preconditions.java:220)
	at org.apache.iceberg.rest.requests.CreateTableRequest$Builder.setProperties(CreateTableRequest.java:154)
	at com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergTable.toCreateTableRequest(IcebergTable.java:57)
	at com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergCatalogOperations.createTable(IcebergCatalogOperations.java:526)
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.lambda$null$30(CatalogOperationDispatcher.java:432)
	at com.datastrato.gravitino.catalog.CatalogManager$CatalogWrapper.lambda$doWithTableOps$1(CatalogManager.java:97)
	at com.datastrato.gravitino.utils.IsolatedClassLoader.withClassLoader(IsolatedClassLoader.java:69)
	at com.datastrato.gravitino.catalog.CatalogManager$CatalogWrapper.doWithTableOps(CatalogManager.java:92)
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.lambda$createTable$31(CatalogOperationDispatcher.java:430)
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.doWithCatalog(CatalogOperationDispatcher.java:696)
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.createTable(CatalogOperationDispatcher.java:427)
	at com.datastrato.gravitino.server.web.rest.TableOperations.lambda$createTable$1(TableOperations.java:96)
	at java.security.AccessController.doPrivileged(Native Method)
	at javax.security.auth.Subject.doAs(Subject.java:422)
	at com.datastrato.gravitino.utils.PrincipalUtils.doAs(PrincipalUtils.java:25)
	at com.datastrato.gravitino.server.web.Utils.doAs(Utils.java:110)
	at com.datastrato.gravitino.server.web.rest.TableOperations.createTable(TableOperations.java:88)

How to reproduce

  1. set table_comment = null
  2. run CatalogIcebergIT#testCreateAndLoadIcebergTable

Additional context

No response
@FANNG1
Copy link
Contributor Author

FANNG1 commented Jan 17, 2024

please check JDBC table too, @Clearvive

@FANNG1 FANNG1 mentioned this issue Jan 17, 2024
Merged
@jerryshao jerryshao added this to the Gravitino 0.4.0 milestone Jan 19, 2024
@FANNG1 FANNG1 self-assigned this Jan 23, 2024
@FANNG1 FANNG1 mentioned this issue Jan 23, 2024
Merged
jerryshao pushed a commit that referenced this issue Jan 24, 2024
@FANNG1
[#1546] fix(lakehouse-iceberg): fix create Iceberg table failed if co…
f29cc7b 
…mment is null (#1653)

### What changes were proposed in this pull request?
remove reserveProperties logic from CreateTableRequest since reserved
properties is handled by property system

### Why are the changes needed?

1. when build IcebergTable, comment equals to null is checked when
comment added to properties .
```
      if (null != comment) {
        icebergTable.properties.putIfAbsent(ICEBERG_COMMENT_FIELD_NAME, comment);
      }
```
2. when build CreateTableRequest, the previous implement remove all
reserved properties from property map including comment, and then add
comment to map, the bug happens, not checking the null, but this overall
code is redundant


Fix: #1546 

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
add UT
mchades pushed a commit to mchades/gravitino that referenced this issue Jan 24, 2024
@FANNG1 @mchades
[apache#1546] fix(lakehouse-iceberg): fix create Iceberg table failed…
c21c90e 
… if comment is null (apache#1653)

### What changes were proposed in this pull request?
remove reserveProperties logic from CreateTableRequest since reserved
properties is handled by property system

### Why are the changes needed?

1. when build IcebergTable, comment equals to null is checked when
comment added to properties .
```
      if (null != comment) {
        icebergTable.properties.putIfAbsent(ICEBERG_COMMENT_FIELD_NAME, comment);
      }
```
2. when build CreateTableRequest, the previous implement remove all
reserved properties from property map including comment, and then add
comment to map, the bug happens, not checking the null, but this overall
code is redundant


Fix: apache#1546 

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
add UT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@FANNG1 FANNG1

@Clearvive Clearvive

Labels
None yet
Projects
None yet
Milestone
Gravitino 0.4.0
Development

Successfully merging a pull request may close this issue.

[#1546] fix(lakehouse-iceberg): fix create Iceberg table failed if comment is null FANNG1/gravitino
3 participants
@jerryshao @FANNG1 @Clearvive