Skip to content

csvim: the IDENTITY restart after a seed import is skipped for every tenant schema - SAFE_IDENTIFIER rejects the schema name the platform itself generates #7435

Description

@iliyan-velichkov

What happens

A CSV seed import always writes explicit primary keys, which leaves the table's IDENTITY counter at
its initial value. CsvimProcessor knows this and repairs it right after the load:

// CsvimProcessor.java:240-247
updateSequence(csvFile, connection, countAll);
if (countAll > 0) {
    // CSV inserts always carry an explicit PK value; the DB's IDENTITY counter is therefore
    // not advanced by the load. Bump it to MAX(col)+1 so a subsequent INSERT … DEFAULT
    // (e.g. from a Hibernate @GeneratedValue(IDENTITY) entity) doesn't collide on the seeded
    // rows.
    restartIdentityColumns(dataSource, connection, targetSchema, tableName);
}

But restartIdentityColumns refuses to run unless the schema and table names match an allow-list:

// CsvimProcessor.java:304-311
private void restartIdentityColumns(DirigibleDataSource dataSource, Connection connection, String schema, String tableName) {
    if (!SAFE_IDENTIFIER.matcher(String.valueOf(schema)).matches()
            || !SAFE_IDENTIFIER.matcher(String.valueOf(tableName)).matches()) {
        logger.warn("Skipping IDENTITY restart — schema/table name not a safe SQL identifier: [{}].[{}]", …);
        return;
    }

// CsvimProcessor.java:471
private static final Pattern SAFE_IDENTIFIER = Pattern.compile("[A-Za-z_][A-Za-z0-9_]*");

A tenant schema can never match that pattern. The platform assigns a tenant a random UUID as its
id (TenantEndpoint.java:108tenant.setId(UUID.randomUUID().toString())) and derives the schema
name from it (DefaultDataSourceProvisioning.java:166-169return tenant.getId().toUpperCase()).
A UUID fails on the hyphens, and two times out of three on the leading digit as well. So for every
tenant the platform provisions itself, the counter is left where the load found it.

Observed on PostgreSQL, with the schema and table names replaced by neutral equivalents — the only
difference between the two lines is a hyphen in the schema name:

[WARN] [tenant-initialization] CsvimProcessor - Skipping IDENTITY restart — schema/table name not a safe SQL identifier: [my-tenant].[ORDERS]
[INFO] [tenant-initialization] CsvimProcessor - Advanced IDENTITY counter on [mytenant.ORDERS.ORDER_ID] to [7]

The consequence

The first row a user creates through a generated UI or API in that tenant asks Hibernate's
identity generator for a value, PostgreSQL hands back 1, and the insert dies on a seeded row:

ERROR: duplicate key value violates unique constraint "ORDERS_pkey"
  Detail: Key ("ORDER_ID")=(1) already exists.

What makes this expensive to diagnose is that it heals itself. A failed insert still consumes a
value, so each retry moves one id further — 1, 2, 3 … — until the counter clears MAX(seeded) and a
create finally succeeds. With six seeded rows the user sees six failures and then normal behaviour,
which reads as flakiness rather than as a deterministic defect. The only durable signal is the WARN,
and nothing surfaces it to the person doing the creating.

This is the tenant-schema remainder of #3482, which reported the same collision for the default
schema. restartIdentityColumns closes that case; it does not reach this one.

Why the default tenant is fine, and why CI stays green

PUBLIC matches SAFE_IDENTIFIER, so in the default tenant the restart runs and everything works.
The regression test is pinned to exactly that case — CsvimIdentityRestartIT.java:87 declares
"schema": "PUBLIC" and asserts the next generated id is 6 — and there is no multitenant variant
anywhere in tests/. The one path that works is the only one covered.

Two sibling guards fail the same way

csvSuppliesIdentityColumn (:416-423) and setMssqlIdentityInsert (:448-455) carry the
identical SAFE_IDENTIFIER test and are therefore also dead on a tenant schema. On MSSQL that is
worse than a stale counter: without SET IDENTITY_INSERT … ON the engine rejects an explicit value
for an identity column outright, so the seed import itself fails rather than merely leaving the
counter behind.

Suggested fix

Quote the identifiers instead of allow-listing them. The allow-list was written for a world of
unquoted identifiers and was never reconciled with the platform's own schema naming, while the sinks
already quote:

  • PostgreSQL (:390-395) emits setval(pg_get_serial_sequence('"<schema>"."<table>"', '<column>'), n, false)
    — already inner-quoted, and would work unchanged for a UUID schema.
  • H2 (:386-389) already builds its DDL with connection.getMetaData().getIdentifierQuoteString().
  • MySQL/MariaDB (:399-400) already uses backticks.
  • MSSQL (:396-398) passes '<schema>.<table>' to DBCC CHECKIDENT unquoted, so that one needs
    [schema].[table].

The residual injection risk is a quote character inside a name, which doubling handles. If the
allow-list is kept instead, it must at minimum accept what the platform's own provisioning
generates.

Whatever the mechanism, a skipped restart should not be a WARN that only a log reader ever sees when
the tenant it applies to is about to serve traffic.

Acceptance criteria

  • After a seed import into a non-default tenant schema, an INSERT that omits the identity column
    receives MAX(seeded)+1, not 1.
  • A multitenant variant of CsvimIdentityRestartIT covers it — the current test cannot fail on this.
  • Identifiers reach the DDL correctly quoted per dialect, and no identifier is concatenated into SQL
    unquoted; injection safety is preserved without rejecting legitimate names.
  • csvSuppliesIdentityColumn and setMssqlIdentityInsert work on the same names, so an MSSQL seed
    import into a tenant schema succeeds.
  • A skip remains possible only for a dialect with no supported restart form, and says so.

Evidence

claim file:line
the restart is called after every non-empty load components/data/data-csvim/src/main/java/org/eclipse/dirigible/components/data/csvim/processor/CsvimProcessor.java:240-247
…and bails out on the allow-list same file :304-311
the allow-list same file :471
the four dialect branches same file :375-409
the two sibling guards same file :416-423, :448-455
CSV inserts carry explicit PKs components/data/data-csvim/src/main/java/org/eclipse/dirigible/components/data/csvim/processor/CsvProcessor.java:95-113
a tenant id is a random UUID components/core/core-tenants/src/main/java/org/eclipse/dirigible/components/tenants/endpoint/TenantEndpoint.java:108
the tenant schema name is that id, uppercased components/data/data-sources/src/main/java/org/eclipse/dirigible/components/data/sources/provisioning/DefaultDataSourceProvisioning.java:166-169
a modelled generated: true PK becomes a native identity column on PostgreSQL components/data/data-structures/src/main/java/org/eclipse/dirigible/components/data/structures/synchronizer/table/TableCreateProcessor.java:88, modules/database/database-sql-postgres/src/main/java/org/eclipse/dirigible/database/sql/dialects/postgres/PostgresSqlDialect.java:115-117
the generated entity asks the DB for it components/data/data-store-java/src/main/java/org/eclipse/dirigible/components/data/store/java/hbm/JavaEntityToHbmMapper.java:183-189
the test pins the default schema tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/CsvimIdentityRestartIT.java:87

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions