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.

Sign up for GitHub

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

Jump to bottom

Wrong column type for PLAN_HINT in statements summary #56057

Open
xhebox opened this issue Sep 13, 2024 · 5 comments · May be fixed by #60331
Open

Wrong column type for PLAN_HINT in statements summary #56057

xhebox opened this issue Sep 13, 2024 · 5 comments · May be fixed by #60331
Assignees
@henrybw
Labels
severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@xhebox
Copy link
Contributor

xhebox commented Sep 13, 2024

Bug Report

tidb/pkg/infoschema/tables.go

Line 1393 in d6db1e8

{name: stmtsummary.PlanHint, tp: mysql.TypeVarchar, size: 64, comment: "Sampled plan hint"},

It should be text, PLAN_HINT may be large. I encountered this when developing workload repository.

1. Minimal reproduce step (Required)

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiDB version? (Required)

master-09-13
@xhebox xhebox added the type/bug The issue is confirmed as a bug. label Sep 13, 2024
@hawkingrei hawkingrei assigned hawkingrei and unassigned hawkingrei Sep 13, 2024
@henrybw
Copy link
Contributor

henrybw commented Sep 17, 2024

/assign

@tiancaiamao
Copy link
Contributor

Do you mean size: 64 is too small for this column?
Maybe notice that enlarge value might cause chunk size too big and increase the Go GC burden.
Ref #28339

@xhebox
Copy link
Contributor Author

xhebox commented Sep 20, 2024

Do you mean size: 64 is too small for this column? Maybe notice that enlarge value might cause chunk size too big and increase the Go GC burden. Ref #28339

Yeah, it is too small. There is no good workaround, since it will output much longer texts than the column definition. This works because that we don't check column types for internal mem table like this one.

@henrybw
Copy link
Contributor

henrybw commented Mar 29, 2025

This works because that we don't check column types for internal mem table like this one.

I think this is a deeper issue that needs to be addressed, as I suspect there are other bugs like this lurking in the codebase undetected.

@henrybw henrybw mentioned this issue Mar 29, 2025
Open
@henrybw
Copy link
Contributor

henrybw commented Mar 29, 2025

This works because that we don't check column types for internal mem table like this one.

I think this is a deeper issue that needs to be addressed, as I suspect there are other bugs like this lurking in the codebase undetected.

Filed a separate issue to track the more general problem: #60330.

@henrybw henrybw linked a pull request Mar 29, 2025 that will close this issue
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@henrybw henrybw

Labels
severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

stmtsummary: change plan_hint column type to text henrybw/tidb
5 participants
@henrybw @tiancaiamao @hawkingrei @jebter @xhebox