-
Notifications
You must be signed in to change notification settings - Fork 319
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
[#2401] improvement(spark-connector): support alter namespace operation #2407
Conversation
cc @FANNG1. |
@SteNicholas thanks for your work, could you add related sql test in SparkIT? please refer to https://spark.apache.org/docs/latest/sql-ref-syntax-ddl-alter-database.html |
@SteNicholas Hi, maybe @PengleiShi is doing this issue, too. This issue has been assigned to him before your pull request. |
9aa8e57
to
33c7e4a
Compare
@FANNG1, I have added |
spark-connector/src/main/java/com/datastrato/gravitino/spark/catalog/GravitinoCatalog.java
Outdated
Show resolved
Hide resolved
integration-test/src/test/java/com/datastrato/gravitino/integration/test/spark/SparkIT.java
Outdated
Show resolved
Hide resolved
33c7e4a
to
46e5fac
Compare
spark-connector/src/main/java/com/datastrato/gravitino/spark/catalog/GravitinoCatalog.java
Outdated
Show resolved
Hide resolved
3832758
to
1523a89
Compare
1523a89
to
b3ba47c
Compare
@FANNG1, I have addressed above comments. PTAL. |
integration-test/src/test/java/com/datastrato/gravitino/integration/test/spark/SparkIT.java
Outdated
Show resolved
Hide resolved
LGTM except one comment |
@SteNicholas could you change the Spark SQL keyword to upper case? since #2437 is merged first. |
@FANNG1, I will update this pull request tonight and change the Spark SQL keyword to upper case. |
b3ba47c
to
e18af1f
Compare
@FANNG1, I have addressed above comment and changed Spark SQL keyword to upper case. PTAL. |
integration-test/src/test/java/com/datastrato/gravitino/integration/test/spark/SparkIT.java
Outdated
Show resolved
Hide resolved
integration-test/src/test/java/com/datastrato/gravitino/integration/test/spark/SparkIT.java
Outdated
Show resolved
Hide resolved
some minor comments, could you fix it? |
merged to main, @SteNicholas thanks for your work! |
What changes were proposed in this pull request?
Support alter namespace operation implementation.
Why are the changes needed?
Support to set namespace properties in Spark sql like:
Fix: #2401
Does this PR introduce any user-facing change?
No.
How was this patch tested?
SparkIT#testAlterSchema