Skip to content

fix(model): H2 quoting follow-ups from #4628 (migrator DDL, tenant JOIN memo key, classifier coverage) #4629

Open

Description

@bpamiri

Follow-ups from the independent review of #4628 (H2 identifier quoting, closed issue #4440). None blocks the merged fix; each is a separate, scoped change.

  1. H2Migrator DDL quoting. H2Migrator.quoteTableName()/quoteColumnName() (vendor/wheels/migrator/H2Migrator.cfc:93-101) still emit unquoted names, so a migration cannot create a reserved-word column on H2 even though the model layer can now query one. PostgreSQL, Oracle and CockroachDB migrators have the same gap, so this wants one decision covering all folded-DDL adapters rather than an H2-only patch.

  2. Association JOIN memo key and datasource-dependent quoting. sql.cfm:2370 memoises the JOIN fragment under sd{n}_alias{n} in application-scoped class data. That fragment now contains H2 quoting that depends on the effective datasource's fold case, so two H2 tenant datasources with different DATABASE_TO_UPPER/DATABASE_TO_LOWER settings could be served each other's memoised JOIN. It fails loudly (wrong-case identifier not found), but the key should include the fold case or effective datasource for H2.

  3. Classifier unit coverage. $readIdentifierCase() (vendor/wheels/databaseAdapters/H2/H2Model.cfc:102-112) has no direct test: the unit tests inject identifierCase through H2CaseProbe and bypass the classifier, and the only real assertion is "upper" on the suite datasource. Extracting a pure $classifyIdentifierCase(settings) and table-testing the verified shapes (2.x default, 2.x DATABASE_TO_UPPER=FALSE, 2.x DATABASE_TO_LOWER=TRUE, 1.3.x with no DATABASE_TO_LOWER) would pin it. Related: add the 1.3 evidence to the comment at H2Model.cfc:102-105 — the "no DATABASE_TO_LOWER ⇒ upper" shortcut is only correct because H2 1.3 compares quoted identifiers case-insensitively, and without that note it reads like a bug.

  4. Spec fixture hygiene. vendor/wheels/tests/specs/model/OrderReservedWordSpec.cfc:109 runs DELETE FROM c_o_r_e_casepreservation with no WHERE. Harmless today (nothing seeds that fixture), but scope it to the ids the spec creates if the fixture ever gains data.

Evidence: review at task wheels-7; H2 semantics verified against H2 1.3.172, 2.1.214, 2.2.224 and 2.3.232.

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