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
[SPARK-37200][SQL] Support drop index for Data Source V2 #34469
Conversation
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #144850 has finished for PR 34469 at commit
|
Test build #144856 has finished for PR 34469 at commit
|
Kubernetes integration test unable to build dist. exiting with code: 1 |
Kubernetes integration test starting |
Kubernetes integration test status failure |
Don't we need JIRA number? |
Added Jira number. @viirya |
Test build #144857 has finished for PR 34469 at commit
|
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.
LGTM with two nits. Maybe the title should be "Support drop index for Data Source V2"?
@@ -162,9 +162,26 @@ class MySQLIntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTest { | |||
assert(jdbcTable.indexExists("i1") == true) | |||
assert(jdbcTable.indexExists("i2") == true) | |||
|
|||
// This should pass without Exception |
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.
nit: Exception
-> exception
.
table.dropIndex(indexName) | ||
} catch { | ||
case _: NoSuchIndexException if ignoreIfNotExists => | ||
logWarning(s"Index $indexName not exists. Ignoring.") |
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.
nit: not exists
-> does not exist
test("DROP INDEX") { | ||
parseCompare("DROP index i1 ON a.b.c", | ||
DropIndex(UnresolvedTable(Seq("a", "b", "c"), "DROP INDEX", None), "i1", false)) | ||
} |
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 add another "IF EXISTS" case?
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #144865 has finished for PR 34469 at commit
|
What changes were proposed in this pull request?
add drop index syntax
Why are the changes needed?
so user can drop index using sql
Does this PR introduce any user-facing change?
yes
new sql syntax
DROP INDEX [IF EXISTS] index_name ON [TABLE] table_name
How was this patch tested?
new test