Skip to content

ddl: fix TypeBit default value#9467

Merged
ti-chi-bot[bot] merged 12 commits intopingcap:masterfrom
Lloyd-Pottiger:fix-bit-default
Sep 26, 2024
Merged

ddl: fix TypeBit default value#9467
ti-chi-bot[bot] merged 12 commits intopingcap:masterfrom
Lloyd-Pottiger:fix-bit-default

Conversation

@Lloyd-Pottiger
Copy link
Contributor

@Lloyd-Pottiger Lloyd-Pottiger commented Sep 24, 2024

What problem does this PR solve?

Issue Number: close #9461

Problem Summary:

What is changed and how it works?

Fix two problems:

  1. When the default value of TypeBit contains invalid characters (like 0x07 or 0x08), TiFlash fails to parse the schema JSON.
  2. The default value of TypeBit is origin_default_bit_value.
ddl: fix TypeBit default value 

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Fix TiFlash fails to parse the table schema when the schema contains a TypeBit column with a default value that represents an invalid character.
修复当表里含 Bit 类型列并且带有表示非法字符的默认值时 TiFlash 无法解析表 schema 的问题。

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 24, 2024
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 24, 2024
@Lloyd-Pottiger Lloyd-Pottiger changed the title ddl: fix bit default value ddl: fix TypeBit default value Sep 24, 2024
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@CalvinNeo
Copy link
Member

Why we need to update Poco here?

@Lloyd-Pottiger
Copy link
Contributor Author

Why we need to update Poco here?

Check pingcap/poco#22

Comment on lines -159 to -166
// TODO: We shall use something like `orig_default_bit`, which will never change once created,
// rather than `default_bit`, which could be altered.
// See https://github.com/pingcap/tidb/issues/17641 and https://github.com/pingcap/tidb/issues/17642
const auto & bit_value = default_bit_value;
// TODO: There might be cases that `orig_default` is not null but `default_bit` is null,
// i.e. bit column added with an default value but later modified to another.
// For these cases, neither `orig_default` (may get corrupted) nor `default_bit` (modified) is correct.
// This is a bug anyway, we choose to make it simple, i.e. use `default_bit`.
Copy link
Contributor

Choose a reason for hiding this comment

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

origin_default_bit_value is added since which version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Sep 25, 2024
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@Lloyd-Pottiger
Copy link
Contributor Author

/cc @JaySon-Huang @JinheLin @CalvinNeo

@ti-chi-bot ti-chi-bot bot requested a review from JaySon-Huang September 26, 2024 05:26
@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Sep 26, 2024
Copy link
Member

@CalvinNeo CalvinNeo left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CalvinNeo, JaySon-Huang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [CalvinNeo,JaySon-Huang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Sep 26, 2024
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Sep 26, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-09-26 07:33:39.666107222 +0000 UTC m=+1724089.406531161: ☑️ agreed by JaySon-Huang.
  • 2024-09-26 07:40:11.83171883 +0000 UTC m=+1724481.572142767: ☑️ agreed by CalvinNeo.

@ti-chi-bot ti-chi-bot bot merged commit 1ce2f11 into pingcap:master Sep 26, 2024
@ti-chi-bot ti-chi-bot bot added needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. and removed needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. labels Sep 27, 2024
ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Sep 27, 2024
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.5: #9478.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Sep 27, 2024
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-8.1: #9479.

@Lloyd-Pottiger Lloyd-Pottiger deleted the fix-bit-default branch September 27, 2024 06:38
ti-chi-bot bot pushed a commit that referenced this pull request Sep 27, 2024
close #9461

ddl: fix TypeBit default value

Signed-off-by: JaySon-Huang <tshent@qq.com>

Co-authored-by: JaySon-Huang <tshent@qq.com>
ti-chi-bot bot pushed a commit that referenced this pull request Sep 30, 2024
close #9461

ddl: fix TypeBit default value

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

Co-authored-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Co-authored-by: JaySon-Huang <tshent@qq.com>
@Lloyd-Pottiger
Copy link
Contributor Author

/cherry-pick release-7.1

@Lloyd-Pottiger
Copy link
Contributor Author

/cherry-pick release-6.5

@ti-chi-bot

This comment was marked as resolved.

@ti-chi-bot
Copy link
Member

@Lloyd-Pottiger: failed to apply #9467 on top of branch "release-6.5":

[failed to git add conflicting files: exit status 128, failed to git commit: exit status 128]
Details

In response to this:

/cherry-pick release-6.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Oct 29, 2024
@ti-chi-bot

This comment was marked as resolved.

ti-chi-bot bot pushed a commit that referenced this pull request Nov 4, 2024
close #9461

Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed to parse TiDB schema with binary default value

4 participants