Skip to content

[FLINK-38244][hotfix] Fix the full snapshot phase, the field case is adjusted based on isTableIdCaseInsensitive#4284

Open
sd4324530 wants to merge 6 commits intoapache:masterfrom
sd4324530:hotfix-flink-38244
Open

[FLINK-38244][hotfix] Fix the full snapshot phase, the field case is adjusted based on isTableIdCaseInsensitive#4284
sd4324530 wants to merge 6 commits intoapache:masterfrom
sd4324530:hotfix-flink-38244

Conversation

@sd4324530
Copy link
Contributor

During the full snapshot phase, the field case is adjusted based on isTableIdCaseInsensitive.

@sd4324530
Copy link
Contributor Author

@leonardBang @loserwang1024 Please take a look when you have time.

@ThorneANN
Copy link
Contributor

looks good but no test

@sd4324530
Copy link
Contributor Author

looks good but no test

@ThorneANN I've tested it in a real environment and I'm already using it. Because the test module has not been working properly in my local environment

@ThorneANN
Copy link
Contributor

ThorneANN commented Feb 27, 2026

looks good but no test

@ThorneANN I've tested it in a real environment and I'm already using it. Because the test module has not been working properly in my local environment

test it class without your local env ,just write some test code and use maven and docker to run test applition

@sd4324530
Copy link
Contributor Author

sd4324530 commented Feb 27, 2026

looks good but no test

@ThorneANN I've tested it in a real environment and I'm already using it. Because the test module has not been working properly in my local environment

test it class without your local env ,just write some test code and use maven and docker to run test applition

#4275
After this pr, I found that my local Docker environment is now ready for testing. I will add test cases as soon as possible.

@lvyanquan
Copy link
Contributor

MySQL field name case sensitivity should be independent of table name case sensitivity and should not be affected by
the lower_case_table_names configuration parameter.

@sd4324530
Copy link
Contributor Author

sd4324530 commented Mar 4, 2026

MySQL field name case sensitivity should be independent of table name case sensitivity and should not be affected by the lower_case_table_names configuration parameter.

@lvyanquan You're right. This issue was discussed in the DingTalk group before. The lower_case_table_names configuration option is used to control the case of table names, not fields. But the previous pr-4095 modified the incremental phase, causing inconsistencies between the full and incremental phases when fields are uppercase. After communication with @loserwang1024 , the conclusion was to first handle the full phase with the same rules, and then consider a unified solution later, such as converting all fields to lowercase. What do you think?

@loserwang1024
Copy link
Contributor

the conclusion was to first handle the full phase with the same rules, and then consider a unified solution later, such as converting all fields to lowercase.

@lvyanquan , yes, we first make the behavior of #4095 same, and then can move forward in another pr.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes MySQL pipeline connector snapshot/schema handling so that column (and PK) name casing is normalized consistently during full snapshot/schema emission when isTableIdCaseInsensitive is enabled.

Changes:

  • Add an uppercase_products fixture table (uppercase column names) to the MySQL inventory test DDL.
  • Expand IT coverage to run the existing alter-statement parsing test against both products and uppercase_products.
  • Normalize emitted schema column/PK names to lower-case in MySqlPipelineRecordEmitter when table IDs are case-insensitive, and plumb the flag from MySqlDataSource.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.../src/test/resources/ddl/inventory.sql Adds uppercase_products table + data to exercise uppercase column-name scenarios.
.../MySqlTableIdCaseInsensitveITCase.java Parameterizes the existing IT to validate schema-change parsing for both normal and uppercase-column tables.
.../MySqlPipelineITCase.java Excludes uppercase_products from an exclusion test to keep expected outputs stable.
.../MySqlDataSourceFactoryTest.java Updates expected discovered tables list to include uppercase_products.
.../MySqlPipelineRecordEmitter.java Lowercases column names and primary key column names when isTableIdCaseInsensitive is true.
.../MySqlDataSource.java Computes isTableIdCaseInsensitive once and passes it to both deserializer and record emitter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

(default,"hammer","14oz carpenter's hammer",0.875),
(default,"hammer","16oz carpenter's hammer",1.0),
(default,"rocks","box of assorted rocks",5.3),
(default,"jacket","water resistent black wind breaker",0.1),
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

Typo in inserted test data: "water resistent" is misspelled and should be "water resistant" (even if this is only test DDL, keeping strings correct avoids propagating typos across fixtures).

Copilot uses AI. Check for mistakes.
Comment on lines +269 to 273
String colName =
this.isTableIdCaseInsensitive
? column.name().toLowerCase(Locale.ROOT)
: column.name();
DataType dataType =
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The new case-normalization logic for column names/primary keys is gated by isTableIdCaseInsensitive, but there doesn't appear to be test coverage for the bounded snapshot (StartupOptions.snapshot()) path where schemas are fetched via SHOW CREATE TABLE/DESC. Consider adding an IT that runs with StartupOptions.snapshot() against a table with uppercase column names (and PK) to exercise this end-to-end.

Copilot uses AI. Check for mistakes.
…justed based on `isTableIdCaseInsensitive`.

Signed-off-by: peiyu <125331682@qq.com>
Signed-off-by: Pei Yu <125331682@qq.com>
Signed-off-by: Pei Yu <125331682@qq.com>
Signed-off-by: Pei Yu <125331682@qq.com>
Signed-off-by: Pei Yu <125331682@qq.com>
@sd4324530 sd4324530 force-pushed the hotfix-flink-38244 branch from 6b34990 to a5f8680 Compare March 5, 2026 14:52
Signed-off-by: Pei Yu <125331682@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants