feat(mysql): add support for MySQL 9 / MariaDB 12 #11575
Conversation
WalkthroughIntroduces MySQL/MariaDB “latest” CI job and a dedicated ormconfig; updates existing MySQL/MariaDB versions in CI and docker-compose; refactors MySQL/Aurora drivers to rename width-related types/sets to unsigned-focused equivalents; gates width behavior and locking semantics by server version; deprecates width/zerofill options; updates docs/examples removing int(11); adjusts and adds tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Svc as MySQL/MariaDB Services
participant Test as Test Runner
participant Cov as Coveralls
Dev->>GH: Push/PR
GH->>GH: Start jobs: mysql_mariadb, mysql_mariadb_latest
GH->>Svc: Launch MySQL/MariaDB containers (versions per job)
GH->>Test: Checkout, setup Node, install deps, download build
Test->>Svc: Run migrations/tests with ormconfig (latest/non-latest)
Test-->>Cov: Upload coverage (flagged, parallel)
sequenceDiagram
autonumber
participant App as App/CLI
participant QR as MysqlQueryRunner
participant DB as INFORMATION_SCHEMA
App->>QR: loadTables(tableNames)
QR->>DB: Query COLUMNS for tables
DB-->>QR: COLUMN_TYPE, DATA_TYPE, etc.
QR->>QR: Parse COLUMN_TYPE
note over QR: zerofill = includes("zerofill")<br/>unsigned = zerofill OR includes("unsigned")<br/>width types = driver.unsignedColumnTypes
QR-->>App: Table metadata (with version-gated width handling)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
Deploying typeorm with
|
| Latest commit: |
1b081f9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1911713a.typeorm.pages.dev |
| Branch Preview URL: | https://feat-mysql9.typeorm.pages.dev |
alumni
force-pushed
the
feat/mysql9
branch
5 times, most recently
from
25bd587 to
0870a5d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
docker-compose.yml (2)
14-24: Fix port conflict between MySQL services.Both
mysql-5andmysql-9are configured to use port 3306, which will cause conflicts when running both services simultaneously.mysql-9: image: "mysql:9.3.0" container_name: "typeorm-mysql-9" ports: - - "3306:3306" + - "3308:3306" environment: MYSQL_ROOT_PASSWORD: "admin" MYSQL_USER: "test" MYSQL_PASSWORD: "test" MYSQL_DATABASE: "test"
37-47: Fix port conflict between MariaDB services.Both
mariadb-10andmariadb-12are configured to use port 3307, which will cause conflicts when running both services simultaneously.mariadb-12: image: "mariadb:12.0.1-rc" container_name: "typeorm-mariadb-12" ports: - - "3307:3306" + - "3309:3306" environment: MYSQL_ROOT_PASSWORD: "admin" MYSQL_USER: "test" MYSQL_PASSWORD: "test" MYSQL_DATABASE: "test"
🧹 Nitpick comments (5)
src/driver/mysql/MysqlQueryRunner.ts (1)
3390-3394: Only doc-comment changed – consider short-circuiting the deprecated code pathThe new
@deprecatedJSDoc correctly informs users that MySQL no longer supports column width.
As a (very minor) follow-up, you might add an early‐return guard inisDefaultColumnWidthfor MySQL ≥ 8/MariaDB ≥ 10.6 so the method exits immediately, e.g.:+ // Skip the whole check on servers that no longer expose width metadata. + if (VersionUtils.isGreaterOrEqual(this.driver.version, "8.0.0")) { + return true + }Purely an optional micro-optimisation; the current implementation remains functionally correct.
test/functional/database-schema/column-types/mysql/uuid.ts (1)
30-34: Clarify the comment about ID persistence issues.The comment mentions "an issue with the persisting id that crosses over from mysql to mariadb" but doesn't explain what the issue is or why this specific UUID value resolves it.
Consider expanding the comment to explain the specific issue:
- // seems there is an issue with the persisting id that crosses over from mysql to mariadb + // Use a specific UUID to avoid ID persistence issues when running tests across MySQL and MariaDBtest/github-issues/8832/issue-8832.ts (3)
16-16: Minor: Remove unnecessary empty line.Consider removing this trailing empty line for consistency.
-
28-30: Fix typo in comment.There's a typo in the comment that should be corrected.
- // inet6 avialable since 10.5 + // inet6 available since 10.5
123-149: Good validation test with room for improvement.The test correctly validates that invalid configurations throw errors, but consider these improvements:
- More specific error validation: Instead of just expecting any Error, consider asserting on specific error messages or types.
- Version setting impact: Manually setting
connection.driver.version = "10.10.0"might affect other tests if connections are reused.Consider this improvement for more specific error validation:
entityMetadatas.forEach((entityMetadata) => { - expect(() => - entityMetadataValidator.validate( - entityMetadata, - entityMetadatas, - connection.driver, - ), - ).to.throw(Error) + expect(() => + entityMetadataValidator.validate( + entityMetadata, + entityMetadatas, + connection.driver, + ), + ).to.throw(/length.*not.*supported|length.*invalid/) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.github/workflows/test/mysql-mariadb-latest.ormconfig.json(1 hunks).github/workflows/tests-linux.yml(2 hunks)docker-compose.yml(4 hunks)docs/docs/data-source/2-data-source-options.md(2 hunks)docs/docs/entity/1-entities.md(0 hunks)docs/docs/getting-started.md(1 hunks)docs/docs/help/3-decorator-reference.md(2 hunks)ormconfig.sample.json(2 hunks)package.json(1 hunks)src/decorator/columns/Column.ts(3 hunks)src/decorator/options/ColumnNumericOptions.ts(1 hunks)src/decorator/options/ColumnOptions.ts(2 hunks)src/decorator/options/ColumnWithSignOptions.ts(1 hunks)src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts(1 hunks)src/driver/mysql/MysqlConnectionOptions.ts(1 hunks)src/driver/mysql/MysqlDriver.ts(0 hunks)src/driver/mysql/MysqlQueryRunner.ts(1 hunks)src/driver/spanner/SpannerConnectionOptions.ts(0 hunks)src/driver/spanner/SpannerDriver.ts(0 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts(2 hunks)test/functional/database-schema/column-collation/mysql/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts(1 hunks)test/functional/database-schema/column-types/mysql/uuid.ts(1 hunks)test/functional/database-schema/column-width/mysql/column-width.ts(1 hunks)test/functional/driver/mysql/connection-options/enable-query-timeout.ts(2 hunks)test/functional/query-builder/locking/query-builder-locking.ts(2 hunks)test/functional/repository/find-options-locking/find-options-locking.ts(3 hunks)test/github-issues/1839/entity/Category.ts(1 hunks)test/github-issues/1839/entity/Post.ts(1 hunks)test/github-issues/1839/issue-1839.ts(3 hunks)test/github-issues/2096/issue-2096.ts(1 hunks)test/github-issues/3702/issue-3702.ts(3 hunks)test/github-issues/4753/issue-4753.ts(2 hunks)test/github-issues/6442/issue-6442.ts(1 hunks)test/github-issues/8747/entity/Record.ts(1 hunks)test/github-issues/8747/issue-8747.ts(1 hunks)test/github-issues/8832/issue-8832.ts(1 hunks)test/unit/util/import-utils.ts(10 hunks)
💤 Files with no reviewable changes (4)
- src/driver/mysql/MysqlDriver.ts
- src/driver/spanner/SpannerDriver.ts
- src/driver/spanner/SpannerConnectionOptions.ts
- docs/docs/entity/1-entities.md
🧰 Additional context used
🧬 Code Graph Analysis (12)
test/functional/database-schema/column-width/mysql/column-width.ts (2)
test/utils/test-utils.ts (1)
createTestingConnections(388-482)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/github-issues/8747/issue-8747.ts (1)
test/utils/test-utils.ts (1)
createTestingConnections(388-482)
test/github-issues/1839/entity/Post.ts (2)
src/decorator/columns/PrimaryColumn.ts (1)
PrimaryColumn(37-100)src/decorator/relations/ManyToMany.ts (1)
ManyToMany(33-77)
test/functional/database-schema/column-collation/mysql/entity/Post.ts (1)
src/decorator/columns/Column.ts (1)
Column(134-220)
test/github-issues/8747/entity/Record.ts (1)
src/decorator/columns/Column.ts (1)
Column(134-220)
test/functional/database-schema/column-types/mysql/uuid.ts (2)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)reloadTestingDatabases(500-505)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/github-issues/3702/issue-3702.ts (2)
test/utils/test-utils.ts (2)
createTestingConnections(388-482)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts (1)
src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/github-issues/6442/issue-6442.ts (2)
test/utils/test-utils.ts (1)
createTestingConnections(388-482)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/github-issues/2096/issue-2096.ts (2)
test/utils/test-utils.ts (1)
getTypeOrmConfig(218-220)src/driver/mysql/MysqlConnectionOptions.ts (1)
MysqlConnectionOptions(10-164)
test/functional/repository/find-options-locking/find-options-locking.ts (1)
src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
src/decorator/columns/Column.ts (3)
src/driver/types/ColumnTypes.ts (1)
WithSignColumnType(81-86)src/decorator/options/ColumnCommonOptions.ts (1)
ColumnCommonOptions(6-68)src/decorator/options/ColumnWithSignOptions.ts (1)
ColumnWithSignOptions(4-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (18) / sap
🔇 Additional comments (68)
test/unit/util/import-utils.ts (1)
23-23: LGTM! Modernized filesystem API usage.The migration from the deprecated
fs.rmdirtofs.rmwithforce: trueoption is a good modernization that makes the test cleanup more robust. Theforce: trueoption ensures cleanup succeeds even if directories don't exist, which is ideal for test scenarios.Also applies to: 44-44, 64-64, 85-85, 100-100, 116-116, 133-133, 149-149, 159-159, 178-178
docs/docs/getting-started.md (1)
774-780: LGTM! Documentation updated to reflect modern MySQL behavior.The change from
int(11)tointcorrectly reflects that MySQL 8.0+ ignores display width for integer types. This documentation update aligns well with the broader effort to support newer MySQL versions.package.json (1)
145-145: LGTM! Patch version update for mysql2 dependency.The update from
^3.14.0to^3.14.2is a patch version bump that likely includes bug fixes and improvements. This aligns well with adding support for newer MySQL and MariaDB versions.test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts (1)
1-1: Import path verification successful.The updated path
../../../../../../srccorrectly reaches the project’ssrcdirectory, and bothEntityandPrimaryGeneratedColumnare re-exported insrc/index.ts. No further changes required.test/functional/database-schema/column-collation/mysql/entity/Post.ts (1)
13-13: LGTM! Charset updated to utf8mb4 for better Unicode support.Changing from
utf8toutf8mb4is a best practice asutf8mb4supports full 4-byte Unicode characters (including emojis) while MySQL'sutf8is limited to 3-byte sequences. This aligns well with MySQL 8.0+ which usesutf8mb4as the default charset.ormconfig.sample.json (1)
6-6: connectorPackage is supported at runtime and in typings – no changes neededI’ve verified that:
MysqlConnectionOptions(src/driver/mysql/MysqlConnectionOptions.ts) definesconnectorPackage?: "mysql" | "mysql2".- In
MysqlDriver.loadDependencies()(src/driver/mysql/MysqlDriver.ts), the code readsthis.options.connectorPackage ?? "mysql"and passes it toPlatformTools.load(), with proper fallback logic and error handling.- Documentation (docs/data-source/2-data-source-options.md) already describes the option.
All configs relying on
connectorPackagewill be consumed as expected.test/github-issues/8747/issue-8747.ts (1)
18-18: 👍 Correct glob pattern – avoids accidental miss of.tsentities
*.{js,ts}is the proper brace-expansion syntax; the previous*{.js,.ts}was incorrect.
This fix ensures both JS and TS entity files are loaded in the test harness.test/github-issues/8747/entity/Record.ts (1)
15-16: LGTM: Property reordering has no functional impact.The reordering of the
timestampproperty belowcarUuidis a cosmetic change that doesn't affect the entity's functionality or database schema.test/github-issues/1839/entity/Category.ts (2)
7-7: LGTM: Consistent property ordering in decorator options.Reordering
charsetbeforecollationin the@PrimaryColumndecorator improves consistency across the codebase.
10-10: LGTM: Cleaner ManyToMany decorator syntax.Removing the unused
typeparameter simplifies the decorator syntax while maintaining the same functionality.test/github-issues/1839/entity/Post.ts (2)
7-7: LGTM: Test charset/collation configuration update.The change from
utf8tolatin2charset and corresponding collation update appears to be part of the test scenario for validating different character set configurations.
10-10: LGTM: Consistent ManyToMany decorator syntax improvement.The simplified decorator syntax aligns with the corresponding change in the Category entity and improves code readability.
test/functional/database-schema/column-width/mysql/column-width.ts (3)
10-10: LGTM: Necessary import for version checking.Adding
DriverUtilsimport is required for the MySQL version compatibility logic below.
17-17: LGTM: Expanded driver support for comprehensive testing.Including both
mariadbandmysqldrivers aligns with the PR objective of supporting newer database versions.
20-37: Excellent version-aware test implementation.The logic properly handles the deprecation of column width in MySQL 8.0+ by:
- Checking driver type and version using
DriverUtils.isReleaseVersionOrGreater- Destroying incompatible connections
- Filtering to keep only initialized connections
This ensures tests run only on versions that support the column width feature.
src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts (1)
25-27: Excellent deprecation documentation with migration guidance.The deprecation warning properly:
- Uses the
@deprecatedJSDoc tag for tooling support- Explains the reason (newer MySQL versions no longer support ZEROFILL)
- Provides clear migration guidance using MySQL's recommended
LPADfunction approachThis aligns well with the PR objective of supporting MySQL 9 and MariaDB 12.
test/functional/driver/mysql/connection-options/enable-query-timeout.ts (2)
17-17: LGTM: MariaDB driver inclusion aligns with PR objectives.Good addition of MariaDB driver support alongside MySQL, which is consistent with the broader goal of this PR to expand MySQL/MariaDB compatibility testing.
44-46: LGTM: Simplified error assertion maintains functionality.The direct property access to
err.driverError?.codeis cleaner and more readable while preserving the same validation logic for the protocol sequence timeout error..github/workflows/test/mysql-mariadb-latest.ormconfig.json (1)
1-26: LGTM: Well-structured configuration for latest MySQL/MariaDB testing.The configuration properly sets up both MySQL 9.3 and MariaDB 12.0.1-rc testing with appropriate port separation and the recommended
mysql2connector package. The standard testing credentials and database names are appropriate for CI environments.test/github-issues/6442/issue-6442.ts (3)
12-12: LGTM: Necessary import for version checking.The
DriverUtilsimport is required for the version compatibility logic added below.
18-21: LGTM: Consistent with PR objectives for MariaDB support.Adding MariaDB driver alongside MySQL aligns with the broader MySQL/MariaDB compatibility expansion in this PR.
23-40: LGTM: Proper handling of MySQL 8+ column width deprecation.The conditional logic correctly identifies and removes MySQL 8.0+ connections since column width is no longer supported in these versions. This ensures the test only runs against compatible MySQL versions where column width functionality is still available.
The approach using
DriverUtils.isReleaseVersionOrGreateris appropriate for version checking, and filtering to retain only initialized connections maintains test integrity.docs/docs/data-source/2-data-source-options.md (2)
89-89: LGTM: Clear documentation for the new connector package option.The documentation accurately describes the
connectorPackageoption and appropriately recommendsmysql2for MySQL 8.0+, which aligns with the configuration changes throughout the codebase.
103-103: LGTM: Improved documentation clarity.Consolidating charset and collation into a single line improves readability while maintaining the essential information about default collation behavior.
src/decorator/options/ColumnOptions.ts (2)
28-31: LGTM: Proper deprecation warning for width property.The deprecation notice correctly informs users that the
widthproperty is no longer supported in newer MySQL versions and provides a helpful migration path using character columns with theLPADfunction, as recommended by MySQL.
111-114: LGTM: Consistent deprecation warning for zerofill property.The deprecation notice for
zerofillmatches the approach used for thewidthproperty and provides the same helpful migration guidance. This ensures consistency across deprecated MySQL column options.src/driver/types/ColumnTypes.ts (2)
81-86: LGTM! Semantic improvement aligns with MySQL evolution.The renaming from
WithWidthColumnTypetoWithSignColumnTypebetter reflects the current reality wherewidthandzerofilloptions are deprecated in newer MySQL versions. The new name accurately describes what these integer types actually support (signed/unsigned attributes).
220-220: Consistent type union update.The
ColumnTypeunion properly reflects the renamed type alias.src/decorator/options/ColumnWithSignOptions.ts (1)
4-27: Excellent deprecation strategy with clear migration guidance.The interface renaming and property deprecations are well-executed:
- Backward compatibility is maintained by keeping deprecated properties
- Clear deprecation messages explain the reasoning (newer MySQL versions don't support these features)
- Actionable alternatives are provided (character columns + LPAD function)
- The
unsignedproperty remains valid, which is correctThis approach gives users time to migrate while providing clear guidance on the recommended alternatives.
docs/docs/help/3-decorator-reference.md (2)
137-137: Documentation properly reflects deprecation status.The deprecation notice for the
widthoption is clear and aligns with the code changes.
153-153: Comprehensive deprecation guidance for zerofill option.The deprecation notice provides both the reasoning and a practical alternative (character column + LPAD function), which helps users understand both why the feature is being removed and how to achieve similar functionality.
test/github-issues/4753/issue-4753.ts (3)
1-1: Modern import style improves readability.The destructured import of
expectfrom chai is cleaner than importing the entire library.
30-37: Excellent refactor improves code clarity.The destructuring of connection options makes it much clearer which specific properties are used for replication configuration, and the spread operator usage is more concise than repeating properties.
42-43: Updated to modern DataSource API.Using
initialize()instead ofconnect()and the chai assertion style are both improvements that align with current best practices.src/decorator/columns/Column.ts (3)
8-8: Import updated consistently with type renaming.The import reflects the renaming from
WithWidthColumnTypetoWithSignColumnType.
20-20: Import updated consistently with interface renaming.The import reflects the renaming from
ColumnWithWidthOptionstoColumnWithSignOptions.
68-70: Function overload signature properly updated.The
Columndecorator overload now uses the renamed types, maintaining consistency across the codebase while preserving the same functionality.test/functional/repository/find-options-locking/find-options-locking.ts (2)
1-1: LGTM: Import reorganization improves readability.The
expectimport moved to the top andDriverUtilsimport added for version checking functionality.Also applies to: 11-11
150-160: Excellent version-aware MySQL locking syntax handling.The conditional logic correctly handles the MySQL 8.0+ change from
"LOCK IN SHARE MODE"to"FOR SHARE"for pessimistic read locks. This ensures compatibility across MySQL versions while using the appropriate syntax.src/decorator/options/ColumnNumericOptions.ts (2)
20-23: Well-documented deprecation with clear migration guidance.The deprecation notice clearly explains that
zerofillis no longer supported in newer MySQL versions and provides the recommended alternative usingLPADfunction.
28-28: Appropriate deprecation for unsigned option.The deprecation correctly reflects MySQL's limited support for unsigned integers in newer versions.
test/github-issues/1839/issue-1839.ts (3)
1-1: Good import optimization.Replacing the generic
reflect-metadataimport with explicitexpectimport improves clarity.
13-13: Excellent: Expanded test coverage to include MariaDB.Adding MariaDB to the enabled drivers aligns with the PR objective of supporting both MySQL 9 and MariaDB 12.
29-36: Charsets and collations match the entity definitionsThe test expectations for
postId(latin2/latin2_general_ci) andcategoryId(ascii/ascii_general_ci) align exactly with the@PrimaryColumnoptions in:
- test/github-issues/1839/entity/Post.ts
- test/github-issues/1839/entity/Category.ts
No further changes required.
test/functional/database-schema/column-types/mysql/uuid.ts (2)
1-24: Well-structured test setup for UUID column type verification.The test properly configures connections for both MySQL and MariaDB drivers with appropriate schema management.
49-66: Excellent version-aware UUID type handling.The logic correctly identifies MariaDB 10.7+ for native UUID support and validates the expected column type and length accordingly. The test assertions properly verify both
DATA_TYPEandCHARACTER_MAXIMUM_LENGTH.test/github-issues/3702/issue-3702.ts (2)
1-3: Improved imports for better clarity.The explicit imports for
expect,DataSource, andDriverUtilsare cleaner and more specific than the previous approach.
31-38: Appropriate version-aware test skipping for legacy spatial support.The conditional skips for MySQL 8.0+ are correctly implemented since legacy spatial functions (
GeomFromText,AsText) are not supported in newer MySQL versions. This ensures tests only run on compatible database versions.Also applies to: 58-65, 84-91
test/github-issues/2096/issue-2096.ts (3)
12-18: LGTM: Improved control flow with early return pattern.The refactoring from
if (mysqlConfig)toif (!mysqlConfig) returnreduces nesting and improves readability without changing the test logic.
20-32: LGTM: Clean destructuring and configuration spreading.The destructuring approach clearly separates connection parameters from the rest of the configuration, and the spread operator usage in the DataSource constructor is more explicit and maintainable than the previous approach.
34-34: LGTM: Consistent assertion method.Changed from
.to.eq(true)to.to.equal(true)which is the preferred chai assertion method for strict equality checks.test/functional/query-builder/locking/query-builder-locking.ts (2)
1-11: LGTM: Improved import organization.The explicit imports improve clarity about dependencies and follow better practices for test file organization.
383-393: LGTM: Correct version-aware locking syntax handling.The conditional logic properly handles the MySQL version difference where 8.0+ uses "FOR SHARE" while earlier versions use "LOCK IN SHARE MODE". This is essential for supporting both legacy and modern MySQL versions, including the new MySQL 9.x support introduced in this PR.
test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts (2)
1-9: LGTM: Clean import organization.The addition of expect from chai and DriverUtils import provides the necessary dependencies for the test improvements.
42-59: LGTM: Correct version-aware charset and collation handling.The conditional logic properly handles the difference in MySQL 8.0+ where charset and collation defaults are handled differently, resulting in undefined values rather than explicit utf8mb4 settings. This ensures test compatibility across MySQL versions, including the new MySQL 9.x support.
.github/workflows/tests-linux.yml (3)
115-115: LGTM: Updated MariaDB version for better compatibility.The MariaDB version update from 10.11.13 to 10.6.22-jammy provides better stability and aligns with the Docker Compose service definitions.
140-141: LGTM: Improved flag naming and parallel execution.The flag name change from
mysql+mariadb-nodetomysql_mariadb-nodefollows better naming conventions, and explicit parallel execution configuration is good practice.
143-183: test:fast script verified for MySQL/MariaDB latest jobI’ve confirmed that
test:fastin package.json is defined as justmocha. Since the workflow pulls in the pre-builtbuild/artifact before runningnpx nyc npm run test:fast, skipping a compile step here is intentional and correct. No changes needed.• .github/workflows/tests-linux.yml (lines 143–183)
• package.json (scripts.test:fast)LGTM.
docker-compose.yml (3)
3-13: LGTM: Clear service naming and configuration.The renaming from
mysqltomysql-5and addition of explicit environment variables improves clarity and configuration management.
26-36: LGTM: Proper MariaDB 10 service configuration.The service configuration for MariaDB 10.6.22-jammy is properly set up with distinct port mapping and environment variables.
49-66: LGTM: Consistent PostgreSQL service naming.The service renaming from
postgres14topostgres-14andpostgrestopostgres-17provides better consistency and clarity in service naming conventions.test/github-issues/8832/issue-8832.ts (7)
4-4: LGTM: DriverUtils import correctly added.The import is necessary for the version checking logic used in the test setup.
17-26: LGTM: Test setup correctly configured for MariaDB-only testing.The connection setup properly filters to MariaDB drivers and disables synchronization initially, which is appropriate for version-aware testing.
31-46: LGTM: Version-aware connection filtering is well implemented.The logic correctly destroys connections that don't meet the minimum MariaDB version requirement (10.10) and only synchronizes supported versions. This ensures tests only run on compatible database versions.
48-52: LGTM: Efficient connection filtering and cleanup.The approach of filtering to only initialized connections after version checking is efficient and the cleanup in the after hook is properly implemented.
54-77: LGTM: Comprehensive test for MariaDB type persistence.The test thoroughly validates:
- Entity persistence with uuid, inet4, and inet6 types
- Data retrieval and comparison
- IPv6 address normalization (line 76 correctly expects the normalized form)
- UUID field generation for
another_uuid_fieldThe test data uses valid values and proper assertions.
79-102: LGTM: Schema validation through INFORMATION_SCHEMA query.The test correctly:
- Queries the database schema to verify column data types
- Maps expected types to actual column names
- Validates that MariaDB correctly created columns with the expected data types
This provides good coverage for ensuring the schema generation works as expected.
104-121: LGTM: Relation testing provides additional coverage.The test extends coverage by validating that entities with the new types can participate in relationships, which is important for ensuring the types work in real-world scenarios.
alumni
force-pushed
the
feat/mysql9
branch
3 times, most recently
from
48e9a4f to
68085e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/tests-linux.yml (2)
143-145: Newmysql_mariadb_latestjob may benefit from the same throttling logicLike the CockroachDB job, this spins up two heavyweight database containers. Adding a guard, e.g.
if: ${{ inputs.node-version >= 20 }}would halve runner minutes when tests are triggered for multiple Node versions.
166-183: Repeated install / copy / test steps are ripe for a reusable actionThe three MySQL/MariaDB jobs share identical steps. Extracting them into a composite action or a
uses:step with parameters would cut 30+ lines and reduce maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
.github/workflows/test/mysql-mariadb-latest.ormconfig.json(1 hunks).github/workflows/tests-linux.yml(3 hunks)docker-compose.yml(4 hunks)docs/docs/drivers/mysql.md(2 hunks)docs/docs/entity/1-entities.md(0 hunks)docs/docs/getting-started.md(1 hunks)docs/docs/help/3-decorator-reference.md(2 hunks)ormconfig.sample.json(2 hunks)package.json(1 hunks)src/decorator/columns/Column.ts(3 hunks)src/decorator/options/ColumnNumericOptions.ts(1 hunks)src/decorator/options/ColumnOptions.ts(2 hunks)src/decorator/options/ColumnWithSignOptions.ts(1 hunks)src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts(1 hunks)src/driver/aurora-mysql/AuroraMysqlConnectionOptions.ts(1 hunks)src/driver/aurora-mysql/AuroraMysqlDriver.ts(2 hunks)src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts(2 hunks)src/driver/mysql/MysqlConnectionOptions.ts(1 hunks)src/driver/mysql/MysqlDriver.ts(2 hunks)src/driver/mysql/MysqlQueryRunner.ts(3 hunks)src/driver/spanner/SpannerConnectionOptions.ts(0 hunks)src/driver/spanner/SpannerDriver.ts(0 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts(2 hunks)test/functional/database-schema/column-collation/mysql/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts(1 hunks)test/functional/database-schema/column-types/mysql/uuid.ts(1 hunks)test/functional/database-schema/column-width/mysql/column-width.ts(1 hunks)test/functional/driver/mysql/connection-options/enable-query-timeout.ts(2 hunks)test/functional/query-builder/locking/query-builder-locking.ts(2 hunks)test/functional/repository/find-options-locking/find-options-locking.ts(3 hunks)test/github-issues/1839/entity/Category.ts(1 hunks)test/github-issues/1839/entity/Post.ts(1 hunks)test/github-issues/1839/issue-1839.ts(3 hunks)test/github-issues/2096/issue-2096.ts(1 hunks)test/github-issues/3702/issue-3702.ts(3 hunks)test/github-issues/4753/issue-4753.ts(2 hunks)test/github-issues/6442/issue-6442.ts(1 hunks)test/github-issues/8747/entity/Record.ts(1 hunks)test/github-issues/8747/issue-8747.ts(1 hunks)test/github-issues/8832/issue-8832.ts(1 hunks)test/unit/util/import-utils.ts(10 hunks)
💤 Files with no reviewable changes (3)
- src/driver/spanner/SpannerDriver.ts
- src/driver/spanner/SpannerConnectionOptions.ts
- docs/docs/entity/1-entities.md
✅ Files skipped from review due to trivial changes (3)
- docs/docs/getting-started.md
- src/driver/aurora-mysql/AuroraMysqlConnectionOptions.ts
- test/github-issues/8747/issue-8747.ts
🚧 Files skipped from review as they are similar to previous changes (35)
- package.json
- test/functional/database-schema/column-collation/mysql/entity/Post.ts
- src/decorator/options/ColumnWithSignOptions.ts
- src/driver/mysql/MysqlConnectionOptions.ts
- test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts
- test/unit/util/import-utils.ts
- test/github-issues/8747/entity/Record.ts
- docs/docs/drivers/mysql.md
- test/github-issues/1839/entity/Category.ts
- test/functional/database-schema/column-types/mysql/uuid.ts
- src/driver/aurora-mysql/AuroraMysqlDriver.ts
- test/functional/driver/mysql/connection-options/enable-query-timeout.ts
- test/functional/database-schema/column-width/mysql/column-width.ts
- test/github-issues/6442/issue-6442.ts
- .github/workflows/test/mysql-mariadb-latest.ormconfig.json
- src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts
- src/driver/types/ColumnTypes.ts
- src/decorator/columns/Column.ts
- src/decorator/options/ColumnOptions.ts
- ormconfig.sample.json
- docs/docs/help/3-decorator-reference.md
- test/github-issues/4753/issue-4753.ts
- test/github-issues/2096/issue-2096.ts
- test/github-issues/1839/issue-1839.ts
- test/functional/query-builder/locking/query-builder-locking.ts
- src/driver/mysql/MysqlDriver.ts
- test/github-issues/1839/entity/Post.ts
- test/functional/repository/find-options-locking/find-options-locking.ts
- test/github-issues/3702/issue-3702.ts
- src/decorator/options/ColumnNumericOptions.ts
- docker-compose.yml
- test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts
- test/github-issues/8832/issue-8832.ts
- src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts
- src/driver/mysql/MysqlQueryRunner.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: tests-linux (20) / sqlite
- GitHub Check: tests-linux (20) / sap
- GitHub Check: tests-linux (20) / postgres (14-3.5)
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (20) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (20) / oracle
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / postgres (14-3.5)
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / mysql_mariadb_latest
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: tests-windows / sqlite
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
.github/workflows/tests-linux.yml (4)
106-108: Patch-level MySQL bump looks fineUpgrading
mysql:5.7.37→5.7.44is a non-breaking patch release and should be safe.
115-118: Downgrading MariaDB from 10.11 → 10.6 may drop feature coverage
10.6is LTS, but you’re removing test coverage for the still-supported10.11series. If users rely on 10.11 features/behaviour, regressions could slip through. Consider keeping 10.11 (or adding a third job) unless there’s a specific incompatibility.
140-141: Coveralls flag rename is 👍Switching to an underscore avoids the special-char ‘+’ that occasionally confuses Coveralls grouping.
147-164: Verified Docker Hub tags existI’ve confirmed that both
mysql:9.4.0(including variants like9.4.0-oracle,9.4.0-oraclelinux9) andmariadb:12.0.1-rcare published in the official Docker Hub library. No changes are needed to the workflow.
alumni
force-pushed
the
feat/mysql9
branch
3 times, most recently
from
9599b94 to
5f89df9
Compare
| constructor(connection: DataSource) { | ||
| this.connection = connection | ||
| this.options = { | ||
| legacySpatialSupport: true, |
There was a problem hiding this comment.
this is a breaking, wdyt about waiting for v1?
There was a problem hiding this comment.
I can also revert it (and only use it in the tests).
Will try to do this when I get some time.
BTW, in v1 we can think about supporting mysql2 only.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/functional/query-builder/locking/query-builder-locking.ts (1)
1045-1061: Gate SKIP LOCKED expectation by server/version (Postgres, MySQL >= 8.0, MariaDB >= 10.6).
MariaDB added SELECT ... SKIP LOCKED in 10.6 and uses LOCK IN SHARE MODE (not MySQL's FOR SHARE). Update the test to run only for: postgres OR (mysql && version >= 8.0.0) OR (mariadb && version >= 10.6.0).
File: test/functional/query-builder/locking/query-builder-locking.ts (lines 1045-1061)
🧹 Nitpick comments (18)
test/github-issues/1839/entity/Category.ts (1)
7-8: Primary key charset/collation: OK; consider pinning length for schema stabilityMySQL 9.3 and MariaDB 12 both include the "ascii" charset and the "ascii_general_ci" collation.
- @PrimaryColumn({ charset: "ascii", collation: "ascii_general_ci" }) + @PrimaryColumn({ type: "varchar", length: 64, charset: "ascii", collation: "ascii_general_ci" })test/github-issues/3702/issue-3702.ts (3)
31-38: Prefer explicit skip over silent pass on MySQL ≥ 8.0.Early-return inside map makes the test “pass” rather than “skip.” Use Mocha’s this.skip() and filter connections instead.
Remove the inner guard here:
- if ( - DriverUtils.isReleaseVersionOrGreater( - connection.driver, - "8.0", - ) - ) { - return - }Then adjust the test (outside this hunk) to a function body and skip/filter:
it("should use GeomFromText", function () { if (connections.every(c => DriverUtils.isReleaseVersionOrGreater(c.driver, "8.0"))) { this.skip() } return Promise.all( connections .filter(c => !DriverUtils.isReleaseVersionOrGreater(c.driver, "8.0")) .map(async (connection) => { // existing test body... }), ) })
58-66: Apply the same explicit-skip pattern here.Mirror the change from the first test: remove the inner guard and switch the test to function () with this.skip() + filtered connections.
- if ( - DriverUtils.isReleaseVersionOrGreater( - connection.driver, - "8.0", - ) - ) { - return - }
84-92: Apply the same explicit-skip pattern to this test as well.Remove the inner guard; prefer this.skip() and filtering at the test level.
- if ( - DriverUtils.isReleaseVersionOrGreater( - connection.driver, - "8.0", - ) - ) { - return - }test/functional/query-builder/locking/query-builder-locking.ts (3)
382-393: Normalize version threshold to 8.0.0 for consistency.Other checks in this file use "8.0.0". Use the same here to avoid parser quirks.
- DriverUtils.isReleaseVersionOrGreater( - connection.driver, - "8.0", - ) + DriverUtils.isReleaseVersionOrGreater( + connection.driver, + "8.0.0", + )
1169-1186: Test name/body mismatch: “skip_locked” vs using nowait.The test title says skip_locked; body uses nowait. Replace with skip_locked.
- .setOnLocked("nowait") + .setOnLocked("skip_locked")
220-236: Deduplicate VERSION() lookups.Same query/parsing repeats. Extract a tiny helper to reduce noise.
// near top of file (test-local helper) async function getServerVersionInfo(conn: DataSource) { const [{ version }] = await conn.query(`SELECT VERSION() as version;`) const v = String(version).toLowerCase() return { raw: v, isMaria: v.includes("maria") } }Then reuse:
const { raw, isMaria } = await getServerVersionInfo(connection) if (!isMaria && VersionUtils.isGreaterOrEqual(raw, "8.0.0")) { /* ... */ }Also applies to: 256-273, 296-319, 340-361
test/github-issues/8832/issue-8832.ts (2)
74-76: Useequalfor string equality (minor)
deep.equalis unnecessary for primitives;equalis clearer.- expect(foundUser!.uuid).to.deep.equal(newUser.uuid) - expect(foundUser!.inet4).to.deep.equal(newUser.inet4) - expect(foundUser!.inet6).to.deep.equal("2001:db8::ff00:42:8329") + expect(foundUser!.uuid).to.equal(newUser.uuid) + expect(foundUser!.inet4).to.equal(newUser.inet4) + expect(foundUser!.inet6).to.equal("2001:db8::ff00:42:8329")
123-149: Tighten the assertion to the intended validation error; consider renaming the test
- Check the specific message (or pattern) to ensure the failure is due to “length not supported”, not some other validation.
- Name mentions only uuid but the test also validates inet4/inet6.
- it("should throw error if mariadb uuid is supported and length is provided to property", async () => + it("should throw when length is provided on uuid/inet4/inet6 (on supported MariaDB versions)", async () => Promise.all( @@ - entityMetadatas.forEach((entityMetadata) => { - expect(() => - entityMetadataValidator.validate( - entityMetadata, - entityMetadatas, - connection.driver, - ), - ).to.throw(Error) - }) + entityMetadatas.forEach((entityMetadata) => { + expect(() => + entityMetadataValidator.validate( + entityMetadata, + entityMetadatas, + connection.driver, + ), + ).to.throw(/does not support length property/i) + })test/github-issues/2096/issue-2096.ts (4)
12-15: Don’t hide possible undefined with a hard type assertion.Using
as MysqlConnectionOptionsmasks the potentialundefinedfromfind. Keep the union or use a user‑defined type guard.Apply:
- const mysqlConfig = config.find( - (c) => c.name === "mysql" && !c.skip, - ) as MysqlConnectionOptions + const mysqlConfig = config.find( + (c) => c.name === "mysql" && !c.skip, + ) as MysqlConnectionOptions | undefined(or replace with a predicate:
(c): c is MysqlConnectionOptions => c.name === "mysql" && !c.skip).
16-18: Prefer explicit skip over silent pass.Early
returnmakes the test pass without signaling a skip. Use Mocha’sthis.skip()instead.Outside this hunk, change the test signature and call skip:
it("should be possible to define a database by connection url for mysql", async function () { // ... if (!mysqlConfig) return this.skip() // ... })
23-24: Escape credentials/db and handle IPv6 hosts in URL.Tiny hardening for edge configs.
- const url = `mysql://${username}:${password}@${host}:${port}/${database}` + const safeHost = host.includes(":") ? `[${host}]` : host + const url = `mysql://${encodeURIComponent(username)}:${encodeURIComponent(password)}@${safeHost}:${port}/${encodeURIComponent(database)}`
25-35: Ensure cleanup with try/finally so connections are always closed.Prevents leaked handles if an assertion throws later.
const dataSource = new DataSource({ ...restConfig, name: "#2096", url, entities: [__dirname + "/entity/*{.js,.ts}"], synchronize: true, type: "mysql", }) - await dataSource.initialize() - expect(dataSource.isInitialized).to.equal(true) - await dataSource.destroy() + try { + await dataSource.initialize() + expect(dataSource.isInitialized).to.equal(true) + } finally { + await dataSource.destroy().catch(() => undefined) + }src/driver/aurora-mysql/AuroraMysqlDriver.ts (1)
767-787: Consider gating integer display width emission for MySQL/MariaDB versions where it’s removed/ignored.
createFullTypestill appends(${column.width})unconditionally. For newer MySQL/MariaDB, integer display width is deprecated/removed and should be suppressed to avoid warnings/diffs. If version detection isn’t available in this driver, ensure the QueryRunner never setswidthfor ints on those versions (and keep BIT(N) unaffected).src/driver/mysql/MysqlDriver.ts (2)
192-201: Rename acknowledged; consider a temporary compatibility alias for withWidthColumnTypes.The public property rename to
unsignedColumnTypesis a breaking API for external tooling/tests referencing the old name. Provide a deprecated getter alias to reduce churn.export class MysqlDriver implements Driver { @@ withLengthColumnTypes: ColumnType[] = [ "char","varchar","nvarchar","binary","varbinary", ] + /** @deprecated Use `unsignedColumnTypes` */ + get withWidthColumnTypes(): ColumnType[] { + // Legacy alias keeps external code working; BIT is intentionally not included. + return this.unsignedColumnTypes as unknown as ColumnType[] + } + /** * Gets list of column data types that support the unsigned attribute by a driver. */ unsignedColumnTypes: UnsignedColumnType[] = [ "tinyint","smallint","mediumint","int","integer","bigint", ]
779-781: Numeric defaults are quoted and forced via toFixed — risk of unintended rounding/format drift.Quoting numbers is harmless for MySQL, but
toFixed(columnMetadata.scale)will truncate decimals whenscaleis undefined (default 0), altering user intent and causing diffs. Only format whenscaleis explicitly a number; otherwise emit the raw number.- if (typeof defaultValue === "number") { - return `'${defaultValue.toFixed(columnMetadata.scale)}'` - } + if (typeof defaultValue === "number") { + return typeof columnMetadata.scale === "number" + ? `'${defaultValue.toFixed(columnMetadata.scale)}'` + : `${defaultValue}` + }src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts (2)
2140-2145: Set unsigned/zerofill before width parsing — good. Duplicate zerofill assignment below can be removed.The early assignment is correct. There’s a second
tableColumn.zerofill = ...at Line 2210; drop it to avoid confusion.- tableColumn.zerofill = - dbColumn["COLUMN_TYPE"].indexOf("zerofill") !== -1
2759-2805: Default-width check: BIT OK; add version gating for Aurora.
- BIT is already handled: both drivers set dataTypeDefaults.bit.width and isDefaultColumnWidth reads connection.driver.dataTypeDefaults[column.type].width (src/driver/mysql/MysqlDriver.ts, src/driver/aurora-mysql/AuroraMysqlDriver.ts).
- MysqlQueryRunner already skips width checks for MySQL ≥ 8.0 (src/driver/mysql/MysqlQueryRunner.ts:isDefaultColumnWidth).
- AuroraMysqlQueryRunner does not include the same version gate; it currently relies on parsing COLUMN_TYPE parentheses (which avoids setting width when parentheses are absent) but that’s brittle—add the same version check for aurora (or use DriverUtils to check server version independent of driver.options.type) in src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts:isDefaultColumnWidth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.github/workflows/test/mysql-mariadb-latest.ormconfig.json(1 hunks).github/workflows/tests-linux.yml(3 hunks)README-zh_CN.md(5 hunks)README_ko.md(5 hunks)docker-compose.yml(4 hunks)docs/docs/drivers/mysql.md(2 hunks)docs/docs/entity/1-entities.md(1 hunks)docs/docs/entity/2-embedded-entities.md(2 hunks)docs/docs/getting-started.md(5 hunks)docs/docs/help/3-decorator-reference.md(2 hunks)docs/docs/relations/2-one-to-one-relations.md(1 hunks)docs/docs/relations/3-many-to-one-one-to-many-relations.md(1 hunks)docs/docs/relations/4-many-to-many-relations.md(1 hunks)ormconfig.sample.json(1 hunks)package.json(1 hunks)src/decorator/columns/Column.ts(2 hunks)src/decorator/options/ColumnNumericOptions.ts(1 hunks)src/decorator/options/ColumnOptions.ts(2 hunks)src/decorator/options/ColumnUnsignedOptions.ts(1 hunks)src/decorator/options/ColumnWithWidthOptions.ts(0 hunks)src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts(1 hunks)src/driver/aurora-mysql/AuroraMysqlDriver.ts(2 hunks)src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts(2 hunks)src/driver/mysql/MysqlDriver.ts(2 hunks)src/driver/mysql/MysqlQueryRunner.ts(3 hunks)src/driver/spanner/SpannerConnectionOptions.ts(0 hunks)src/driver/spanner/SpannerDriver.ts(0 hunks)src/driver/types/ColumnTypes.ts(2 hunks)test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts(2 hunks)test/functional/database-schema/column-collation/mysql/entity/Post.ts(1 hunks)test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts(1 hunks)test/functional/database-schema/column-types/mysql/uuid.ts(1 hunks)test/functional/database-schema/column-width/mysql/column-width.ts(1 hunks)test/functional/driver/mysql/connection-options/enable-query-timeout.ts(2 hunks)test/functional/entity-schema/columns/mysql/columns-mysql.ts(2 hunks)test/functional/query-builder/locking/query-builder-locking.ts(2 hunks)test/functional/repository/find-options-locking/find-options-locking.ts(3 hunks)test/github-issues/1839/entity/Category.ts(1 hunks)test/github-issues/1839/entity/Post.ts(1 hunks)test/github-issues/1839/issue-1839.ts(3 hunks)test/github-issues/2096/issue-2096.ts(1 hunks)test/github-issues/3702/issue-3702.ts(3 hunks)test/github-issues/4753/issue-4753.ts(2 hunks)test/github-issues/6442/issue-6442.ts(1 hunks)test/github-issues/8747/entity/Record.ts(1 hunks)test/github-issues/8747/issue-8747.ts(1 hunks)test/github-issues/8832/issue-8832.ts(1 hunks)test/unit/util/import-utils.ts(10 hunks)
💤 Files with no reviewable changes (3)
- src/driver/spanner/SpannerConnectionOptions.ts
- src/decorator/options/ColumnWithWidthOptions.ts
- src/driver/spanner/SpannerDriver.ts
✅ Files skipped from review due to trivial changes (4)
- src/decorator/options/ColumnOptions.ts
- docs/docs/getting-started.md
- docs/docs/relations/2-one-to-one-relations.md
- src/decorator/options/ColumnNumericOptions.ts
🚧 Files skipped from review as they are similar to previous changes (31)
- test/unit/util/import-utils.ts
- ormconfig.sample.json
- package.json
- test/functional/database-schema/column-collation/mysql/entity/Post.ts
- README-zh_CN.md
- test/github-issues/8747/issue-8747.ts
- test/functional/entity-schema/columns/mysql/columns-mysql.ts
- src/decorator/options/PrimaryGeneratedColumnNumericOptions.ts
- src/decorator/options/ColumnUnsignedOptions.ts
- docs/docs/relations/4-many-to-many-relations.md
- test/functional/database-schema/column-types/mysql/uuid.ts
- .github/workflows/test/mysql-mariadb-latest.ormconfig.json
- test/github-issues/1839/entity/Post.ts
- test/functional/database-schema/column-types/mysql/entity/UuidEntity.ts
- docs/docs/drivers/mysql.md
- docs/docs/entity/2-embedded-entities.md
- src/driver/types/ColumnTypes.ts
- README_ko.md
- test/functional/driver/mysql/connection-options/enable-query-timeout.ts
- test/github-issues/4753/issue-4753.ts
- test/github-issues/1839/issue-1839.ts
- test/functional/repository/find-options-locking/find-options-locking.ts
- test/github-issues/6442/issue-6442.ts
- test/functional/database-schema/column-width/mysql/column-width.ts
- test/github-issues/8747/entity/Record.ts
- docs/docs/help/3-decorator-reference.md
- docs/docs/relations/3-many-to-one-one-to-many-relations.md
- docs/docs/entity/1-entities.md
- src/driver/mysql/MysqlQueryRunner.ts
- .github/workflows/tests-linux.yml
- docker-compose.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-27T20:50:10.364Z
Learnt from: alumni
PR: typeorm/typeorm#11581
File: docs/docs/drivers/postgres.md:23-23
Timestamp: 2025-07-27T20:50:10.364Z
Learning: The correct data source type for Aurora PostgreSQL in TypeORM is `aurora-postgres`, not `aurora-data-api-pg`. The `aurora-data-api-pg` driver was renamed to `aurora-postgres` according to the CHANGELOG.md. This is defined in the DatabaseType union type and AuroraPostgresConnectionOptions interface.
Applied to files:
src/driver/aurora-mysql/AuroraMysqlDriver.tssrc/driver/aurora-mysql/AuroraMysqlQueryRunner.ts
🧬 Code graph analysis (10)
test/github-issues/1839/entity/Category.ts (2)
src/decorator/columns/PrimaryColumn.ts (1)
PrimaryColumn(37-100)src/decorator/relations/ManyToMany.ts (1)
ManyToMany(33-77)
src/driver/aurora-mysql/AuroraMysqlDriver.ts (1)
src/driver/types/ColumnTypes.ts (1)
UnsignedColumnType(81-87)
test/github-issues/3702/issue-3702.ts (2)
test/utils/test-utils.ts (2)
createTestingConnections(388-482)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
src/decorator/columns/Column.ts (3)
src/driver/types/ColumnTypes.ts (1)
UnsignedColumnType(81-87)src/decorator/options/ColumnCommonOptions.ts (1)
ColumnCommonOptions(6-68)src/decorator/options/ColumnUnsignedOptions.ts (1)
ColumnUnsignedOptions(4-30)
test/github-issues/8832/issue-8832.ts (4)
test/utils/test-utils.ts (2)
createTestingConnections(388-482)closeTestingConnections(487-495)src/driver/DriverUtils.ts (1)
DriverUtils(8-297)src/connection/ConnectionMetadataBuilder.ts (1)
ConnectionMetadataBuilder(17-117)src/metadata-builder/EntityMetadataValidator.ts (1)
EntityMetadataValidator(33-362)
src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts (1)
src/driver/types/ColumnTypes.ts (1)
UnsignedColumnType(81-87)
test/github-issues/2096/issue-2096.ts (2)
test/utils/test-utils.ts (1)
getTypeOrmConfig(218-220)src/driver/mysql/MysqlConnectionOptions.ts (1)
MysqlConnectionOptions(10-164)
test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts (1)
src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
test/functional/query-builder/locking/query-builder-locking.ts (1)
src/driver/DriverUtils.ts (1)
DriverUtils(8-297)
src/driver/mysql/MysqlDriver.ts (1)
src/driver/types/ColumnTypes.ts (1)
UnsignedColumnType(81-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (20) / mysql_mariadb_latest
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (20) / sap
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / mysql_mariadb_latest
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / postgres (14-3.5)
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / postgres (17-3.5)
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: tests-windows / sqljs
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (20)
test/github-issues/1839/entity/Category.ts (1)
10-11: ManyToMany type callback simplification LGTMChanging
(type) => Postto() => Postaligns with the decorator signature and avoids a throwaway param. Ensure the corresponding side in Post mirrors this.test/functional/database-schema/column-collation/mysql/column-collation-mysql.ts (5)
1-2: Assertion style + imports look good.Switch to chai’s expect and direct DataSource import are fine.
8-9: Driver/version gating import is appropriate.Importing Post and DriverUtils aligns with the new version-aware assertions.
37-41: Schema assertion fornamecolumn LGTM.Targeted check using
includeis appropriate here.
61-64: Schema assertion fordescriptioncolumn LGTM.Checks are precise and stable.
42-59: Fix test: expect explicit charset for title instead of undefinedPost entity sets @column({ charset: "utf8mb4" }) on title (test/functional/database-schema/column-collation/mysql/entity/Post.ts:13–14). Replace the MySQL‑8 branch assertions with explicit checks: expect(titleCol.charset).to.equal('utf8mb4'); expect(titleCol.collation).to.be.undefined.
Likely an incorrect or invalid review comment.
test/github-issues/3702/issue-3702.ts (3)
6-6: LGTM: explicit import of createTestingConnections.
15-25: LGTM: scoped legacySpatialSupport in suite setup (mysql-only).
1-3: No change required — reflect-metadata is already bootstrapped for tests.test/utils/test-setup.ts imports "reflect-metadata" (line 2) and is used by the test harness; src/index.ts also imports it.
test/functional/query-builder/locking/query-builder-locking.ts (2)
1-11: Imports cleanup looks good.Switch to explicit chai expect and src barrel imports is fine.
20-20: Import fix acknowledged.PostWithVersion import aligns with usage below.
test/github-issues/8832/issue-8832.ts (5)
4-4: Import for version gating looks goodNeeded for MariaDB-version checks in this test.
17-26: Solid test setup scopeMariaDB-only, sync disabled initially, and explicit entities keep the test fast and deterministic.
48-50: Good cleanup of destroyed connectionsFiltering by
isInitializedprevents double-destroy.
52-52: Teardown hook is correctCloses only live connections.
31-46: Verify driver.version is reliably populated before gatingMysqlDriver sets version via queryRunner.getVersion (src/driver/mysql/MysqlDriver.ts) and this test explicitly assigns connection.driver.version = "10.10.0" (test/github-issues/8832/issue-8832.ts:127). The Driver interface marks version as optional and some drivers (e.g., SAP) may not populate it — either guard DriverUtils.isReleaseVersionOrGreater(...) against undefined or ensure the MariaDB connection's driver.version is set before this check.
test/github-issues/2096/issue-2096.ts (2)
20-21: Destructuring looks good; avoids option collisions.
4-4: LGTM — reflect-metadata loaded in test bootstrap (test/utils/test-setup.ts)
test/utils/test-setup.ts imports "reflect-metadata", so the shared test config covers this test.src/driver/aurora-mysql/AuroraMysqlDriver.ts (1)
184-193: unsignedColumnTypes is fine, but QueryRunner width parsing relies on it — ensure BIT(N) isn’t regressed.This list correctly excludes "bit" (BIT isn’t “unsigned”), but AuroraMysqlQueryRunner now uses
unsignedColumnTypesto decide when to parseCOLUMN_TYPEwidth, which will skip BIT(N). See my QueryRunner comment with a concrete fix.src/decorator/columns/Column.ts (1)
68-70: Restore "bit" overload accepting widthDropping WithWidthColumnType removed the overload that allowed width for "bit"; add a dedicated overload so @column("bit", { width: N }) type-checks.
File: src/decorator/columns/Column.ts — add the overload:
export function Column( - type: UnsignedColumnType, - options?: ColumnCommonOptions & ColumnUnsignedOptions, + type: UnsignedColumnType, + options?: ColumnCommonOptions & ColumnUnsignedOptions, ): PropertyDecorator +/** + * Column decorator for MySQL BIT columns which require a width. + */ +export function Column( + type: "bit", + options?: ColumnCommonOptions & Pick<ColumnUnsignedOptions, "width">, +): PropertyDecoratorrg -nP '@column\s*(\s*["'''"]bit["'''"]\s*,' returned no matches in the repo; verify external/consumer usages if any.
| TABLE_SCHEMA = ${connection.driver.database} | ||
| AND TABLE_NAME = 'user' | ||
| AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'anotherUuid')` | ||
|
|
||
| const expectedColumnTypes: Record<string, string> = { | ||
| id: "uuid", | ||
| uuid: "uuid", | ||
| inet4: "inet4", | ||
| inet6: "inet6", | ||
| another_uuid_field: "uuid", | ||
| } | ||
|
|
||
| columnTypes.forEach(({ COLUMN_NAME, DATA_TYPE }) => { | ||
| expect(DATA_TYPE).to.equal(expectedColumnTypes[COLUMN_NAME]) |
There was a problem hiding this comment.
🧩 Analysis chain
Column name mismatch will fail the INFORMATION_SCHEMA assertion
Query filters anotherUuid but expectations and entity access use another_uuid_field. This will yield expectedColumnTypes[ "anotherUuid" ] === undefined and fail.
Apply:
- AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'anotherUuid')`
+ AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'another_uuid_field')`To confirm the entity’s column name, run:
🏁 Script executed:
#!/bin/bash
# Show the User entity definition to verify the column/property name
fd -a 'User.ts' test | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,200p" "{}"'Length of output: 86349
Column name mismatch will fail the INFORMATION_SCHEMA assertion
User entity defines the column another_uuid_field (test/github-issues/8832/entity/User.ts); the query currently filters 'anotherUuid', so expectedColumnTypes['anotherUuid'] will be undefined.
File: test/github-issues/8832/issue-8832.ts Lines: 88-101
Apply:
- AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'anotherUuid')`
+ AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'another_uuid_field')`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TABLE_SCHEMA = ${connection.driver.database} | |
| AND TABLE_NAME = 'user' | |
| AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'anotherUuid')` | |
| const expectedColumnTypes: Record<string, string> = { | |
| id: "uuid", | |
| uuid: "uuid", | |
| inet4: "inet4", | |
| inet6: "inet6", | |
| another_uuid_field: "uuid", | |
| } | |
| columnTypes.forEach(({ COLUMN_NAME, DATA_TYPE }) => { | |
| expect(DATA_TYPE).to.equal(expectedColumnTypes[COLUMN_NAME]) | |
| TABLE_SCHEMA = ${connection.driver.database} | |
| AND TABLE_NAME = 'user' | |
| AND COLUMN_NAME IN ('id', 'uuid', 'inet4', 'inet6', 'another_uuid_field')` | |
| const expectedColumnTypes: Record<string, string> = { | |
| id: "uuid", | |
| uuid: "uuid", | |
| inet4: "inet4", | |
| inet6: "inet6", | |
| another_uuid_field: "uuid", | |
| } | |
| columnTypes.forEach(({ COLUMN_NAME, DATA_TYPE }) => { | |
| expect(DATA_TYPE).to.equal(expectedColumnTypes[COLUMN_NAME]) |
🤖 Prompt for AI Agents
In test/github-issues/8832/issue-8832.ts around lines 88 to 101, the
INFORMATION_SCHEMA query filters for 'anotherUuid' while the actual column is
named 'another_uuid_field', causing expectedColumnTypes['anotherUuid'] to be
undefined; update the SQL IN list to include 'another_uuid_field' (replace
'anotherUuid') so the queried COLUMN_NAME matches the key used in
expectedColumnTypes.
Description of change
Add support for MySQL 9.3 and MariaDB 12 and setup test workflows.
Fixes #8713
Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit