-
Notifications
You must be signed in to change notification settings - Fork 381
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.
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
[#4062] feat(iceberg-rest-server): support RegisterTable for Iceberg REST server #4088
Conversation
45adcf0
to
14eb09a
Compare
8e81f3b
to
0005f70
Compare
@@ -60,7 +60,7 @@ void testLoadCatalog() { | |||
}); | |||
|
|||
Map<String, String> properties = new HashMap<>(); | |||
properties.put(CatalogProperties.URI, "jdbc://0.0.0.0:3306"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new Iceberg version will check the correctness of jdbc uri
@@ -107,10 +108,10 @@ private Response doLoadTable(String name) { | |||
} | |||
|
|||
private Response doUpdateTable(String name, TableMetadata base) { | |||
MetadataUpdate addSchema = new AddSchema(newTableSchema, base.lastColumnId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API changes for new Iceberg Version
@jerryshao please help to review when you have time |
convertToStringList( | ||
sql( | ||
String.format( | ||
"select file from %s.%s.metadata_log_entries", registerDB, registerTableName)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please make all the SQL keywords uppercase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Assertions.assertEquals(ImmutableMap.of("1", "a"), result); | ||
|
||
// insert other data | ||
sql(" INSERT INTO iceberg_rest_table_test.register_foo2 VALUES (2, 'b')"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the whitespace before "INSERT".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@jerryshao @jerqi , comments are addressed, could you help to review again? |
What changes were proposed in this pull request?
Why are the changes needed?
Fix: #4062
Does this PR introduce any user-facing change?
no
How was this patch tested?