Skip to content

Fix ENUM creation; update CockroachDB versions and fix CI - #162

Merged
rafiss merged 7 commits into
masterfrom
rafiss/fix-enum-create-type
Sep 30, 2026
Merged

rafiss merged 7 commits into
masterfrom
rafiss/fix-enum-create-type

Conversation

@rafiss

@rafiss rafiss commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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_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.

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 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.

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:

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.

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:

  • 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 (Telemetry creates error log when using Sequelize without 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

rafiss and others added 2 commits September 29, 2026 12:22
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
rafiss force-pushed the rafiss/fix-enum-create-type branch from ac6da12 to 59a122d Compare September 29, 2026 16:23
rafiss and others added 4 commits September 29, 2026 12:29
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>
@rafiss rafiss changed the title Fix ENUM creation with newer Sequelize v6; update CRDB versions under test Fix ENUM creation; update CockroachDB versions and fix CI Sep 30, 2026
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
rafiss marked this pull request as ready for review September 30, 2026 04:35
@rafiss
rafiss added this pull request to stack #163 September 30, 2026 04:35
@rafiss
rafiss removed this pull request from stack #163 September 30, 2026 04:36
@rafiss
rafiss merged commit 5e2c284 into master Sep 30, 2026
228 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant