Repository navigation
test(lightweight): add end-to-end integration test for ADD PRIMARY KEY and MODIFY COLUMN NOT NULL - #1457
Merged
Conversation
…Y and MODIFY COLUMN NOT NULL Verify end-to-end replication against MySQL and ClickHouse containers: - ADD PRIMARY KEY alone is skipped without producing Code: 62 syntax error. - ADD PRIMARY KEY as first clause does not leave a leading comma. - MODIFY COLUMN ... NOT NULL keeps the ClickHouse column Nullable, avoiding Code: 36 (Please specify DEFAULT expression) and avoiding stream stalls. - Data inserted after the DDLs replicates successfully.
minguyen9988
added a commit
that referenced
this pull request
Sep 18, 2026
* test: fix the two CI failures on 2.11.0 left by #1455 and #1457 java-tests-lightweight: AlterTableModifyColumnIT.testAlterAddPrimaryKeyAndModifyNotNull (added in #1457) fails inside MySQL before anything reaches the connector: java.sql.SQLSyntaxErrorException: Multiple primary key defined at AlterTableModifyColumnIT.testAlterAddPrimaryKeyAndModifyNotNull(AlterTableModifyColumnIT.java:154) Step 3 issues "ALTER TABLE office ADD PRIMARY KEY (office_id), ..." but alter_ddl_modify_column.sql already creates office with "office_id INT PRIMARY KEY", so MySQL rejects the second key and the multi-clause translation the step exists to exercise never runs. Add a keyless table (branch) to the init script and run the step against it, and assert the outcome on the ClickHouse side as well: branch_name stays Nullable(String) (MODIFY ... NOT NULL on an existing column), branch_status is String (ADD COLUMN ... NOT NULL is honored), and the ADD PRIMARY KEY clause emitted nothing. testflows (lightweight, hikari-pool and replicated): the features "alter/modify column not null" and "alter/change column name to new name not null" time out after 100s waiting for DESC to show "x","String" / "x2","String". Since #1455, MODIFY/CHANGE COLUMN ... NOT NULL keeps the existing ClickHouse column Nullable (converting Nullable -> non-Nullable needs a DEFAULT expression, otherwise Code: 36 stalls the stream), so the column now reads Nullable(String). #1455 updated the Java unit assertions to the Nullable form but not these two testflows expectations; align them and record the rationale in the feature docstrings. The 2x4 failing examples across the three testflows jobs were exactly these two features, and no other suite asserts a non-Nullable result after MODIFY/CHANGE ... NOT NULL. * test: make DECIMAL sample values in-range so is_deleted stops flaking The `is deleted different datatypes / check is_deleted with DECIMAL(2,1) NOT NULL` example failed in only one of the four testflows-lightweight jobs on 2.11.0 (x86 replicated), passing in the other three, and failed fast (~6.8s) at the INSERT rather than timing out on replication -- the signature of a bad generated value, not infrastructure flakiness. generate_sample_mysql_value() drew DECIMAL values from round(random.uniform(-(10**(p-s)), 10**(p-s)), s) but DECIMAL(p, s) admits |value| <= (10**p - 1)/10**s, i.e. 9.9 for DECIMAL(2,1), not 10.0. Any draw >= max - 0.5*10**-s rounds to exactly 10**(p-s), one unit of the last place out of range. Under MySQL 8.0 strict mode the INSERT then fails with "Out of range value", which fails the test at random -- about a 0.5% chance per run for DECIMAL(2,1), matching one-in-four jobs. Generate an integer number of least-significant-digit units over the exact valid range [-(10**p - 1), 10**p - 1] and place the decimal point by string. Every value is in range by construction, so the test is deterministic; building the string from the int also avoids scientific notation (which MySQL rejects) for high-scale types and the default Decimal context's 28-digit precision limit for DECIMAL(30, 10). Verified over 300k draws each for DECIMAL(2,1), DECIMAL(30,10), DECIMAL(10,2), DECIMAL(5,0) and DECIMAL(1,1): zero out-of-range, zero scientific-notation. The only callers are is_deleted.py (all DECIMAL datatypes) and multiple_databases.py (INT/VARCHAR only, unaffected).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an end-to-end integration test (AlterTableModifyColumnIT.testAlterAddPrimaryKeyAndModifyNotNull) validating that the DDL translation fixes replicate cleanly against real MySQL and ClickHouse containers without stalling the replication stream:
ADD PRIMARY KEY alone:
MODIFY COLUMN ... NOT NULL on existing Nullable column:
Multi-clause ALTER with ADD PRIMARY KEY as first clause:
Stream Continuity and Data Replication: