Declare every platform-managed column through the DataJoint type system

[dimitri-yatsenko]

Follow-up to #1555, and a response to the review items left open there. Net 18 lines removed.

Why

_prov was declared by hand-written SQL, once per adapter:

# mysql.py     ->  "`_prov` json DEFAULT NULL"
# postgres.py  ->  '"_prov" jsonb DEFAULT NULL'

That restates a mapping the adapter already owns — core_type_to_sql("json") returns exactly json and jsonb. The better precedent was a few lines away in the same function: the _singleton injection, which is also a declaration-time hidden column and goes through the type system.

I followed job_metadata_columns instead, which does the same hand-written thing. Wrong precedent.

What changes

One function, used by both declare.py and deploy.py:

def column_definition(adapter):
    return adapter.format_column_definition(
        name=PROV_ATTRIBUTE,
        sql_type=adapter.core_type_to_sql("json"),
        nullable=True,
        default="DEFAULT NULL",
        comment=PROV_COMMENT,
    )

• provenance_columns is deleted from all three adapters, the @abstractmethod included.

• A new backend needs nothing added — it inherits the column by implementing core_type_to_sql, which it must implement anyway.

• original_type is now recorded. Verified on both backends:

  mysql      : type='json'   original_type='json'  json=True
  postgresql : type='jsonb'  original_type='json'  json=True
  

  Note PostgreSQL: the SQL type is jsonb but original_type is the DataJoint name json. The hand-written form left it None.

  Correction to an earlier version of this description. I claimed this is what jobs.py:210 and table.py:1400 consume and what Public access to hidden attributes (_job_*, _prov): no reader exists #1562's reader will need. Both are wrong: jobs.py:210 iterates target_pk, and hidden attributes are never in the primary key; table.py:1400 is inside describe(), which iterates heading.attributes with hidden excluded. And decoding does not use original_type at all — json, uuid, numeric and dtype are set from the SQL type at heading.py:499-503, so a hidden attribute decodes correctly with or without the comment.

  The comment comes free with the type-system path and makes introspection consistent. It is not load-bearing. The arguments that are: deleting the abstract method, removing the per-adapter duplication, and the backend mapping living in one place.

Review items from #1555 closed here

@ttngu207 — three of the seven, each in the path this touches:

• provenance_columns as a new @abstractmethod made any out-of-tree adapter uninstantiable. You suggested a default return []; deleting the method is better, and this does that.
• Dry-run counters incremented outside the guard, unlike set_replica_identity next door. They now count work actually done; a dry run reports through ddl and details. The test asserts columns_added == 0 on a preview.
• add_prov_column left the cached heading stale. You were right that the manual _init_from_database() in the test was hiding it: _prov stayed invisible to the insert path and every insert silently recorded nothing until the process reconnected. add_prov_column now invalidates the heading, and the test calls no refresh of its own — it forces the lazy reload the same way _has_prov_attribute does, so it exercises the real path.

Still open from your review, not in this PR

• Unmemoized git rev-parse — ~1.9 ms per insert and per _populate1, and I made it unconditional in autopopulate.py where the old path only paid it on success. It belongs in the populate hot path, not here; worth its own PR so the perf change is reviewable on its own.
• Threads. Confirmed: contextvars do not propagate into ThreadPoolExecutor workers, so a fan-out inside make() loses context silently. Documentation, not a code fix — queued for the spec.
• to_arrays() can't read the hidden job-metadata attributes that job-metadata.md documents #1553 — superseded by Public access to hidden attributes (_job_*, _prov): no reader exists #1562, which covers _job_* and _prov together, with Mapping restriction naming a hidden attribute silently drops the predicate #1561 folded in.

Verification

• Full suite 681 passed, 14 skipped, 0 failed across MySQL and PostgreSQL.
• original_type checked on both backends, since that is the behavior change rather than a refactor.
• test_declare.py run alongside, because _singleton goes through the same format_column_definition path this now shares.
• ruff, ruff-format, codespell, mypy pass via pre-commit.

---

Extended: _job_* too (5e83a56)

Following review discussion, this now covers every platform-managed column, not just _prov. _singleton already used the type system; _prov does as of the first commit; the three _job_* columns were the last hand-written SQL.

It fixes a real defect, not only duplication. The hand-written PostgreSQL column was a bare timestamp, which takes PostgreSQL's default microsecond precision, while MySQL got datetime(3). Confirmed against the catalog rather than inferred:

with the fix      ('_job_start_time', 'timestamp without time zone', 3)
hand-written form ('t',               'timestamp without time zone', 6)

core_type_to_sql("datetime(3)") yields timestamp(3), so both backends now declare the precision that reference/specs/job-metadata.md has been documenting all along. Closes #1566 and #1567.

job_metadata_columns is deleted from all three adapters alongside provenance_columns, so an out-of-tree adapter needs neither.

The two adapter tests are rewritten against the new entry point rather than deleted — they were the only place pinning this DDL, and the PostgreSQL one now asserts timestamp(3) explicitly so the precision cannot regress silently.

Extended: the parser half, and the retrofit path

Two later commits finish what the description above called out of scope.

The grammar now spells a hidden attribute; policy forbids a user writing one. attribute_name takes [a-z_] as its first character class, and the underscore check moved out of compile_attribute — which the framework itself calls — into _reject_user_hidden_attributes, run from declare() before anything is appended. That is the single point a user's definition enters the system, so there is no allow_reserved flag to thread through and forget. The user-facing error is unchanged, and tests/unit/test_declare_hidden_attribute.py now pins that it fires from a definition string rather than only from compile_attribute directly.

With the grammar open, the five platform columns live in declare.py as ordinary DataJoint lines and are spliced into the definition before it is parsed:

_singleton      = 1    : bool          # singleton primary key
_job_start_time = null : datetime(3)   # when computation began
_job_duration   = null : float32       # computation duration in seconds
_job_version    = ''   : varchar(64)   # code version
_prov           = null : json          # extrinsic provenance for a row that entered from outside

migrate.add_job_metadata_columns is fixed rather than left alone. The earlier description argued it was out of scope because the module is deprecated and the ALTER is MySQL-only. That reasoning was backwards: being MySQL-only is the defect, not a reason to leave it. Backtick-quoted identifiers are a syntax error on PostgreSQL, so the retrofit had never run there at all, and datetime(3) is not a PostgreSQL type. It now compiles JOB_METADATA_DEFINITION through compile_attribute and quotes through the adapter, as deploy.add_prov_column already does, with the COMMENT ON alongside.

tests/integration/test_migrate_job_metadata.py declares the same Computed table twice — once with the columns, once without them and then migrated — and asserts the catalog cannot tell the two apart. Without the fix, the three PostgreSQL cases fail outright and the MySQL type comparison fails on the missing :type: marker.

Verification

435 unit, 687 integration passed, 14 skipped across MySQL and PostgreSQL. Catalog precision checked on postgres:15; original_type, retrofit equivalence, and continued populate() recording checked on both backends.