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

plan: get correct columns #5846

Merged
merged 7 commits into from
Feb 12, 2018
Merged

Conversation

zimulala
Copy link
Contributor

@zimulala zimulala commented Feb 9, 2018

@@ -181,6 +181,22 @@ func (t *TableInfo) GetPkColInfo() *ColumnInfo {
return nil
}

// Cols returns the columns of the table in public state.
func (t *TableInfo) Cols() []*ColumnInfo {
Copy link
Member

Choose a reason for hiding this comment

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

This will be called many times. Could you pre-calculate this?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can save the publicColumns in TableInfo.

Copy link
Member

Choose a reason for hiding this comment

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

It's only called in ToPB.
Later we can change the order of columns in DDL, then we don't need to allocate memories.

@coocood
Copy link
Member

coocood commented Feb 11, 2018

LGTM

1 similar comment
@shenli
Copy link
Member

shenli commented Feb 12, 2018

LGTM

@shenli
Copy link
Member

shenli commented Feb 12, 2018

/run-all-tests

@zimulala
Copy link
Contributor Author

/run-integration-common-test

@zimulala zimulala added the status/LGT2 Indicates that a PR has LGTM 2. label Feb 12, 2018
@zimulala
Copy link
Contributor Author

/run-integration-common-test

@shenli shenli merged commit 46e2d3e into pingcap:master Feb 12, 2018
@zimulala zimulala deleted the correct-col-cherry-pick branch July 2, 2018 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants