You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "vimko (via GitHub)" <gi...@apache.org> on 2025/05/28 14:07:06 UTC
[PR] Add support for mysql's drop index (`DROP INDEX idx_a ON table_a` and `ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
vimko opened a new pull request, #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865
This PR adds support for mysql's drop index:
- `DROP INDEX idx_a ON table_a`
- `ALTER TABLE table_a DROP INDEX idx_a`
Addresses https://github.com/apache/datafusion-sqlparser-rs/issues/1863
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#discussion_r2131627748
##########
tests/sqlparser_common.rs:
##########
@@ -9132,7 +9132,9 @@ fn test_create_index_with_with_clause() {
#[test]
fn parse_drop_index() {
let sql = "DROP INDEX idx_a";
- match verified_stmt(sql) {
+ // MySql dialect doesn't support `DROP INDEX idx_a`,you need to specify a specific table, please refer:
+ // [MySql](https://dev.mysql.com/doc/refman/8.4/en/drop-index.html)
+ match all_dialects_except(|d| d.is::<MySqlDialect>()).verified_stmt(sql) {
Review Comment:
not sure I followed the intent here, from the code in this PR the parser doesnt seem to have started rejecting any `DROP INDEX` statement variants specifically for mysql, so that I imagined these should still have been passing for mysql?
##########
src/ast/ddl.rs:
##########
@@ -187,6 +187,14 @@ pub enum AlterTableOperation {
DropForeignKey {
name: Ident,
},
+ /// `DROP INDEX <index_name>`
+ ///
+ /// Note: this is a [MySQL]-specific operation.
+ ///
Review Comment:
```suggestion
```
I think we can skip the note, since we're already linking to the mysql docs below
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`DROP INDEX idx_a ON table_a` and `ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#issuecomment-2918876058
Thanks @vimko! fyi there's a similar PR for `DROP INDEX idx_a on table_a` feature, we'll probably look to land that one and rebase this on top of it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for `ALTER TABLE DROP INDEX` [datafusion-sqlparser-rs]
Posted by "vimko (via GitHub)" <gi...@apache.org>.
vimko commented on PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#issuecomment-2959057710
> @vimko could you take a look at the cargo failure when you get the chance?
@iffyio Thank you for the reminder, I have already taken care of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for `ALTER TABLE DROP INDEX` [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "vimko (via GitHub)" <gi...@apache.org>.
vimko commented on code in PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#discussion_r2134498562
##########
src/ast/ddl.rs:
##########
@@ -187,6 +187,14 @@ pub enum AlterTableOperation {
DropForeignKey {
name: Ident,
},
+ /// `DROP INDEX <index_name>`
+ ///
+ /// Note: this is a [MySQL]-specific operation.
+ ///
Review Comment:
Yes, this is more concise, and SQL Server also supports this syntax, so I don't need to specify that only MySQL supports it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for `ALTER TABLE DROP INDEX` [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#issuecomment-2957664809
@vimko could you take a look at the cargo failure when you get the chance?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "vimko (via GitHub)" <gi...@apache.org>.
vimko commented on code in PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#discussion_r2134494593
##########
tests/sqlparser_common.rs:
##########
@@ -9132,7 +9132,9 @@ fn test_create_index_with_with_clause() {
#[test]
fn parse_drop_index() {
let sql = "DROP INDEX idx_a";
- match verified_stmt(sql) {
+ // MySql dialect doesn't support `DROP INDEX idx_a`,you need to specify a specific table, please refer:
+ // [MySql](https://dev.mysql.com/doc/refman/8.4/en/drop-index.html)
+ match all_dialects_except(|d| d.is::<MySqlDialect>()).verified_stmt(sql) {
Review Comment:
That's right, I didn't think it through well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`DROP INDEX idx_a ON table_a` and `ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "vimko (via GitHub)" <gi...@apache.org>.
vimko commented on PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#issuecomment-2919372439
@iffyio Indeed, I didn't notice that. Thank you very much for reminding me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Add support for mysql's drop index (`ALTER TABLE table_a DROP INDEX idx_a`) [datafusion-sqlparser-rs]
Posted by "vimko (via GitHub)" <gi...@apache.org>.
vimko commented on code in PR #1865:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1865#discussion_r2134498562
##########
src/ast/ddl.rs:
##########
@@ -187,6 +187,14 @@ pub enum AlterTableOperation {
DropForeignKey {
name: Ident,
},
+ /// `DROP INDEX <index_name>`
+ ///
+ /// Note: this is a [MySQL]-specific operation.
+ ///
Review Comment:
Yes, this is more concise, so I don't need to specify that only MySQL supports it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org