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

extend syntax: drop table a, b #3398

Closed
wants to merge 3 commits into from

Conversation

acharis
Copy link
Contributor

@acharis acharis commented Nov 17, 2017

@sougou
the other way i thought to do this was to break this out of DDL into a DropTable statement type. this seemed a bit quicker. i'm happy to do that, or anything else you might come up with along those lines, or clean up however you like. we obviously don't need this, but it came up and some of our folks sneered a bit.

Copy link
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Just skimmed through. Couple of comments.

continue
}
tableName := ddl.Table.Name.String()
tableName := ddl.GetFirstTableName()
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be changed to count total rows for all tables.

}
}

func analyzeDDL(ddl *sqlparser.DDL, tables map[string]*schema.Table) *Plan {
// TODO(sougou): Add support for sequences.
plan := &Plan{
PlanID: PlanDDL,
Table: tables[ddl.Table.Name.String()],
Table: tables[ddl.GetFirstTableName()],
Copy link
Contributor

Choose a reason for hiding this comment

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

This part is tough, but the right thing to do is to change Table to Tables here, and iterate on all the tables wherever DDLs are handled. Without this, vttablet will not reload the schema for the rest of the tables.

This change goes deep because the Table info is also used in ACLs, etc. For now, you can change those parts to just use the first entry.

However, doing this will help us eventually fix the issue where we leave this field empty if there are multiple tables in a select.

@sougou
Copy link
Contributor

sougou commented Mar 13, 2018

Can you update this to use the new ACL fields I added recently? I think that was the blocker for this PR.

@acharis acharis closed this Apr 9, 2018
frouioui pushed a commit to planetscale/vitess that referenced this pull request Mar 26, 2024
…client mount command more standard (vitessio#3402)

* backport of 3398

* Fix conflict

Signed-off-by: Matt Lord <mattalord@gmail.com>

---------

Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
systay pushed a commit that referenced this pull request Jul 22, 2024
)

* cherry pick of 14281

* Fix conflict

Signed-off-by: Matt Lord <mattalord@gmail.com>

---------

Signed-off-by: Matt Lord <mattalord@gmail.com>
Co-authored-by: Matt Lord <mattalord@gmail.com>
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.

3 participants