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.

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

[#5547] feat(core): support schema pre event for Gravitino server #5559

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

orenccl
Copy link
Contributor

@orenccl orenccl commented Nov 12, 2024

What changes were proposed in this pull request?

add schema pre event to event listener

Why are the changes needed?

Close: #5547

Does this PR introduce any user-facing change?

no

How was this patch tested?

add Unit Test

private final SchemaInfo createdSchemaRequest;

public CreateSchemaPreEvent(
String user, NameIdentifier identifier, SchemaInfo createdSchemaRequest) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createdSchemaRequest -> createSchemaRequest?

* @return A {@link SchemaInfo} instance encapsulating the comprehensive details of the newly
* created schema.
*/
public SchemaInfo createdSchemaInfo() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createdSchemaInfo -> createSchemaRequest?

@@ -30,7 +30,7 @@ public class AlterTablePreEvent extends TablePreEvent {

public AlterTablePreEvent(String user, NameIdentifier identifier, TableChange[] tableChanges) {
super(user, identifier);
this.tableChanges = tableChanges;
this.tableChanges = tableChanges.clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In post-event, the objects are cloned to prevent the user from changing the object. But in pre-event, I prefer to relax the constraint to allow users to make changes to in inject some custom logic.

@FANNG1
Copy link
Contributor

FANNG1 commented Nov 13, 2024

@orenccl thanks for your PR, LGTM except minor comments.

@orenccl
Copy link
Contributor Author

orenccl commented Nov 13, 2024

Hi @FANNG1
Thanks for the review. I've made the corrections.

@jerryshao jerryshao changed the title [#5547] subtask: support schema pre event for Gravitino server [#5547] feat(core): support schema pre event for Gravitino server Nov 13, 2024
@FANNG1 FANNG1 merged commit 3cd372f into apache:main Nov 13, 2024
26 checks passed
@FANNG1
Copy link
Contributor

FANNG1 commented Nov 13, 2024

@orenccl merged to main, thanks for your contribution!

@orenccl orenccl deleted the subtask/schemaPreEvent branch November 14, 2024 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Subtask] support schema pre event for Gravitino server
2 participants