Repository navigation
Fix ENUM creation; update CockroachDB versions and fix CI - #162
Merged
Merged
Conversation
Newer versions of Sequelize v6 create ENUM types inside a
`DO ... EXCEPTION WHEN duplicate_object` block, so that creating a type
that already exists is not an error. CockroachDB does not support
CREATE TYPE inside a function body, so any model with an ENUM column
failed to sync with:
unimplemented: CREATE TYPE usage inside a function definition is
not supported
This overrides the query generator's pgEnum to use
CREATE TYPE IF NOT EXISTS instead, which has the same effect.
This was originally proposed in
#159.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previously tested versions (v21.1 and v21.2) are long out of support, and their images are no longer available on Docker Hub. Test against the regular (non-innovation) releases that are still supported: v24.3, v25.2, v25.4, and v26.2. The Sequelize integration tests now run against v26.2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rafiss
force-pushed
the
rafiss/fix-enum-create-type
branch
from
September 29, 2026 16:23
ac6da12 to
59a122d
Compare
The `pg` dependency now resolves to a version that requires Node.js 16 or later, so the main test job failed on Node.js 12 with a SyntaxError while loading `pg`. Run CI with Node.js 22, and update the checkout and setup-node actions to v4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sequelize v5 is no longer maintained. Remove it from the CI matrices of both the main tests and the Sequelize integration tests, along with the list of integration tests that were ignored only for v5. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This flag tells Sequelize's tests whether the database rejects
FOR UPDATE on the nullable side of an outer join. CockroachDB used to
allow this, but now rejects it with the same error as PostgreSQL:
FOR UPDATE cannot be applied to the nullable side of an outer join
Sequelize itself does not read this flag, so removing the override
only affects which behavior the Sequelize integration tests expect.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Sequelize integration test jobs have not actually run any tests for a long time, but still reported success: - runTests.js passed the Promise returned by getTestsToIgnore() to Mocha as the grep filter, without awaiting it. Mocha then failed with "self._grep.test is not a function". - The Sequelize v6 source that CI downloads must be built before it can be loaded, but CI installed its dependencies with --ignore-scripts and never built it, so loading Sequelize failed. In both cases the error was an unhandled promise rejection, which Node.js 12 only logged as a warning before exiting with status 0. Await the ignore list, exit with a non-zero status if setting up the tests fails, and build Sequelize after installing its dependencies. Now that the tests run again, ignore the ones that fail on CockroachDB: - Tests that expect SERIAL ids to start at 1, that expect PostgreSQL's statement timeout error message, or that use deferrable constraints. - The bulkCreate conflictWhere upsert tests, which expect RETURNING to return rows in the same order as the inserted values. - A rollback test that does not wait for the rollback to finish, which races with the check for queries still running after each test. - Two logging tests that fail because telemetry queries are logged through the user's logger (#155). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Some Sequelize integration tests create many tables, and the test harness must drop them all within 10 seconds after each test. On CI runners this sometimes took longer, which failed the job. Start CockroachDB with an in-memory store, which made the include/findAll integration tests run about 4x faster locally (101s to 26s) and the main tests about 3x faster. Also disable automatic statistics collection, range merges, and diagnostics reporting, which are background work that the tests don't need. These settings did not measurably change the run time on their own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rafiss
marked this pull request as ready for review
September 30, 2026 04:35
rafiss
added this pull request to stack #163
September 30, 2026 04:35
rafiss
removed this pull request from stack #163
September 30, 2026 04:36
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.
This PR contains the following commits.
Fix ENUM creation with newer versions of Sequelize v6
Newer versions of Sequelize v6 create ENUM types inside a
DO ... EXCEPTION WHEN duplicate_objectblock, so that creating a typethat already exists is not an error. CockroachDB does not support
CREATE TYPE inside a function body, so any model with an ENUM column
failed to sync with:
This overrides the query generator's pgEnum to use
CREATE TYPE IF NOT EXISTS instead, which has the same effect.
This was originally proposed in
#159.
ci: update CockroachDB versions under test
The previously tested versions (v21.1 and v21.2) are long out of
support, and their images are no longer available on Docker Hub.
Test against the regular (non-innovation) releases that are still
supported: v24.3, v25.2, v25.4, and v26.2. The Sequelize integration
tests now run against v26.2.
ci: run tests with Node.js 22
The
pgdependency now resolves to a version that requiresNode.js 16 or later, so the main test job failed on Node.js 12 with a
SyntaxError while loading
pg.Run CI with Node.js 22, and update the checkout and setup-node actions
to v4.
ci: stop testing with Sequelize v5
Sequelize v5 is no longer maintained. Remove it from the CI matrices of
both the main tests and the Sequelize integration tests, along with the
list of integration tests that were ignored only for v5.
Stop overriding lockOuterJoinFailure
This flag tells Sequelize's tests whether the database rejects
FOR UPDATE on the nullable side of an outer join. CockroachDB used to
allow this, but now rejects it with the same error as PostgreSQL:
Sequelize itself does not read this flag, so removing the override
only affects which behavior the Sequelize integration tests expect.
ci: fix Sequelize integration tests never running
The Sequelize integration test jobs have not actually run any tests for
a long time, but still reported success:
Mocha as the grep filter, without awaiting it. Mocha then failed
with "self._grep.test is not a function".
be loaded, but CI installed its dependencies with --ignore-scripts
and never built it, so loading Sequelize failed.
In both cases the error was an unhandled promise rejection, which
Node.js 12 only logged as a warning before exiting with status 0.
Await the ignore list, exit with a non-zero status if setting up the
tests fails, and build Sequelize after installing its dependencies.
Now that the tests run again, ignore the ones that fail on CockroachDB:
statement timeout error message, or that use deferrable constraints.
return rows in the same order as the inserted values.
races with the check for queries still running after each test.
through the user's logger (Telemetry creates error log when using
Sequelizewithout active connections #155).ci: run CockroachDB with an in-memory store
Some Sequelize integration tests create many tables, and the test
harness must drop them all within 10 seconds after each test. On CI
runners this sometimes took longer, which failed the job.
Start CockroachDB with an in-memory store, which made the
include/findAll integration tests run about 4x faster locally (101s to
26s) and the main tests about 3x faster. Also disable automatic
statistics collection, range merges, and diagnostics reporting, which
are background work that the tests don't need. These settings did not
measurably change the run time on their own.
🤖 Generated with Claude Code