Skip to content

Commit 31cc733

Browse files
refactor: declare hidden attributes in DataJoint notation
Platform columns are now written the way a user writes an attribute, and appended to the table definition before it is parsed. The ordinary machinery then does everything -- backend type mapping, the `:type:` comment, and the column-comment bookkeeping PostgreSQL needs for its out-of-line COMMENT ON. No Python construction, and no adapter method. _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 ... _singleton = 1 : bool # singleton primary key Grammar and policy are separated to make that possible. The attribute grammar accepts a leading underscore, so the framework can spell its own columns; a *user* declaring one is refused by `_reject_user_hidden_attributes`, called from declare() on the user's definition before anything is appended. That ordering is the whole mechanism: the guard never sees the framework's lines. Placement matters and is not uniform: - Job metadata and provenance are secondary, so they follow every user attribute. A `---` is inserted when the definition has none, because otherwise a table whose attributes are all primary key takes a nullable hidden column as a nullable key attribute and is rejected. - `_singleton` is the exception: it *is* the primary key, so it goes into the key section of a table that declares none of its own. Detection is by the absence of attribute or foreign-key lines ahead of the separator, so a leading table comment does not mask it. The test for the underscore ban now exercises the user-facing path rather than compile_attribute, which deliberately accepts these names. It gains coverage for per-tier placement -- Entry gets `_prov`, Computed and Imported get job metadata, Lookup, parts and job tables get neither. deploy.add_prov_column no longer reaches into the in-process heading cache. A Heading is memoized from the database, and refreshing it meant poking private state and guessing which class to poke; a deploy operation runs before the workers that write through it, as set_replica_identity does. The requirement is documented instead, and the test reloads the way a deployment would. 681 passed, 14 skipped.
1 parent 5e83a56 commit 31cc733

7 files changed

Lines changed: 247 additions & 179 deletions

File tree

‎src/datajoint/declare.py‎

Lines changed: 90 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
import pyparsing as pp
1414

15-
from . import provenance
1615
from .codecs import lookup_codec
1716
from .condition import translate_attribute
1817
from .errors import DataJointError
@@ -157,7 +156,10 @@ def build_attribute_parser() -> pp.ParserElement:
157156
"""
158157
quoted = pp.QuotedString('"') ^ pp.QuotedString("'")
159158
colon = pp.Literal(":").suppress()
160-
attribute_name = pp.Word(pp.srange("[a-z]"), pp.srange("[a-z0-9_]")).set_results_name("name")
159+
# A leading underscore is permitted by the grammar so the framework can
160+
# declare its own columns in this notation. Whether a *user* may is policy,
161+
# enforced in prepare_declare where a user's definition is parsed.
162+
attribute_name = pp.Word(pp.srange("[a-z_]"), pp.srange("[a-z0-9_]")).set_results_name("name")
161163
data_type = (
162164
pp.Combine(pp.Word(pp.alphas) + pp.SkipTo("#", ignore=quoted))
163165
^ pp.QuotedString("<", end_quote_char=">", unquote_results=False)
@@ -452,6 +454,84 @@ def _covered(candidate: list[str]) -> bool:
452454
)
453455

454456

457+
#: Primary key for a table that declares none of its own.
458+
SINGLETON_DEFINITION = "_singleton = 1 : bool # singleton primary key"
459+
460+
461+
def _reject_user_hidden_attributes(definition) -> None:
462+
"""Refuse a user-declared attribute whose name begins with an underscore.
463+
464+
Policy, not grammar: the parser accepts such a name so that the framework can
465+
declare its own columns in the same notation. Only a definition written by a
466+
user passes through here.
467+
"""
468+
lines = definition.split("\n") if isinstance(definition, str) else definition
469+
for line in lines:
470+
stripped = line.strip()
471+
if not stripped or stripped.startswith("#") or stripped.startswith("---"):
472+
continue
473+
if is_foreign_key(stripped):
474+
continue
475+
if stripped.startswith("_"):
476+
raise DataJointError(
477+
f'Attribute name in line "{line}" starts with an underscore. '
478+
"Names with leading underscore are reserved for platform-managed "
479+
"columns (e.g. _job_start_time, _singleton). Use a regular "
480+
"attribute name; if you need to control visibility at the call "
481+
"site, use proj()."
482+
)
483+
484+
485+
def _append_platform_attributes(definition, table_name: str, config) -> str:
486+
"""Add the framework's own hidden attributes to a table definition.
487+
488+
They are written in DataJoint notation and parsed by the same machinery as
489+
any user attribute, so the backend type mapping, the ``:type:`` comment and
490+
the column-comment bookkeeping all come from the one path that owns them.
491+
492+
Placement matters. Job metadata and provenance are secondary, so they go
493+
after every user attribute and need a ``---`` ahead of them -- without one,
494+
a definition whose attributes are all primary key would take a nullable
495+
hidden column as a nullable key attribute and be rejected. ``_singleton`` is
496+
the exception: it *is* the primary key, so it goes into the key section of a
497+
table that declares none of its own.
498+
"""
499+
from .jobs import JOB_METADATA_DEFINITION
500+
from .provenance import PROV_DEFINITION
501+
from .user_tables import Manual
502+
503+
lines = list(definition) if not isinstance(definition, str) else definition.split("\n")
504+
505+
def is_attribute(line: str) -> bool:
506+
stripped = line.strip()
507+
return bool(stripped) and not stripped.startswith("#") and not stripped.startswith("---")
508+
509+
separator = next((i for i, line in enumerate(lines) if line.strip().startswith("---")), None)
510+
key_lines = lines[:separator] if separator is not None else lines
511+
512+
secondary = []
513+
# Computed (__) and Imported (_) tables, but not a part (__ in the middle).
514+
is_computed = table_name.startswith("__") and "__" not in table_name[2:]
515+
is_imported = table_name.startswith("_") and not table_name.startswith("__")
516+
if config.jobs.add_job_metadata and (is_computed or is_imported):
517+
secondary.extend(JOB_METADATA_DEFINITION)
518+
519+
# Entry tables, where rows enter from outside. Matched against the Manual
520+
# tier itself rather than by excluding the other tiers' prefixes.
521+
if config.provenance.capture and re.fullmatch(Manual.tier_regexp, table_name):
522+
secondary.append(PROV_DEFINITION)
523+
524+
# A table that declares no primary key of its own gets the sentinel.
525+
singleton = [] if any(is_attribute(line) for line in key_lines) else [SINGLETON_DEFINITION]
526+
527+
if not singleton and not secondary:
528+
return definition if isinstance(definition, str) else "\n".join(lines)
529+
530+
if separator is None:
531+
return "\n".join(key_lines + singleton + ["---"] + secondary)
532+
return "\n".join(lines[:separator] + singleton + lines[separator:] + secondary)
533+
534+
455535
def declare(
456536
full_table_name: str, definition: str, context: dict, adapter, *, config=None
457537
) -> tuple[str, list[str], list[str], dict[str, tuple[str, str]], list[str], list[str]]:
@@ -498,6 +578,14 @@ def declare(
498578
)
499579
)
500580

581+
if config is None:
582+
from .settings import config as _config
583+
584+
config = _config
585+
586+
_reject_user_hidden_attributes(definition)
587+
definition = _append_platform_attributes(definition, table_name, config)
588+
501589
(
502590
table_comment,
503591
primary_key,
@@ -509,53 +597,6 @@ def declare(
509597
column_comments,
510598
) = prepare_declare(definition, context, adapter)
511599

512-
# Add hidden job metadata for Computed/Imported tables (not parts)
513-
if config is None:
514-
from .settings import config as _config
515-
516-
config = _config
517-
if config.jobs.add_job_metadata:
518-
# Check if this is a Computed (__) or Imported (_) table, but not a Part (contains __ in middle)
519-
is_computed = table_name.startswith("__") and "__" not in table_name[2:]
520-
is_imported = table_name.startswith("_") and not table_name.startswith("__")
521-
if is_computed or is_imported:
522-
# Deferred import: jobs imports table, which imports this module.
523-
from .jobs import JOB_METADATA_SPEC, job_metadata_column_definitions
524-
525-
attribute_sql.extend(job_metadata_column_definitions(adapter))
526-
for name, core_type, _default, comment in JOB_METADATA_SPEC:
527-
column_comments[name] = f":{core_type}:{comment}"
528-
529-
# Add the hidden extrinsic-provenance slot to Entry tables, where rows enter
530-
# from outside the pipeline. Computed and Imported tables have no use for
531-
# it -- their provenance is entailed by the foreign-key graph -- and a part
532-
# inherits its master's.
533-
# Matched against the Manual tier itself, not by excluding the other tiers'
534-
# prefixes: enumerating exclusions makes every tier added later an Entry
535-
# table by default, which is how job tables (`~`) first acquired the slot.
536-
# Imported here rather than at module scope: user_tables imports table,
537-
# which imports this module.
538-
from .user_tables import Manual
539-
540-
if config.provenance.capture and re.fullmatch(Manual.tier_regexp, table_name):
541-
attribute_sql.append(provenance.column_definition(adapter))
542-
column_comments[provenance.PROV_ATTRIBUTE] = provenance.PROV_COMMENT
543-
544-
if not primary_key:
545-
# Singleton table: add hidden sentinel attribute
546-
primary_key = ["_singleton"]
547-
singleton_comment = ":bool:singleton primary key"
548-
sql_type = adapter.core_type_to_sql("bool")
549-
singleton_sql = adapter.format_column_definition(
550-
name="_singleton",
551-
sql_type=sql_type,
552-
nullable=False,
553-
default="NOT NULL DEFAULT TRUE",
554-
comment=singleton_comment,
555-
)
556-
attribute_sql.insert(0, singleton_sql)
557-
column_comments["_singleton"] = singleton_comment
558-
559600
pre_ddl = [] # DDL to run BEFORE CREATE TABLE (e.g., CREATE TYPE for enums)
560601
post_ddl = [] # DDL to run AFTER CREATE TABLE (e.g., COMMENT ON)
561602

@@ -964,14 +1005,6 @@ def compile_attribute(
9641005
DataJointError
9651006
If syntax is invalid, primary key is nullable, or blob has invalid default.
9661007
"""
967-
if line.lstrip().startswith("_"):
968-
raise DataJointError(
969-
f'Attribute name in line "{line}" starts with an underscore. '
970-
"Names with leading underscore are reserved for platform-managed "
971-
"columns (e.g. _job_start_time, _singleton). Use a regular "
972-
"attribute name; if you need to control visibility at the call "
973-
"site, use proj()."
974-
)
9751008
try:
9761009
match = attribute_parser.parse_string(line + "#", parse_all=True)
9771010
except pp.ParseException as err:

‎src/datajoint/deploy.py‎

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -186,30 +186,6 @@ def set_replica_identity(
186186
return result
187187

188188

189-
def _refresh_heading(target, table_name: str) -> None:
190-
"""Drop a cached heading so a freshly added column is visible in-process.
191-
192-
`Heading` loads from the database once and memoizes. After an ALTER the
193-
cached copy is stale, and for `_prov` that is silent: `_has_prov_attribute`
194-
reports False and inserts record nothing.
195-
"""
196-
from .schemas import _Schema
197-
198-
candidates = []
199-
if isinstance(target, _Schema):
200-
context = getattr(target, "context", None) or {}
201-
candidates = [cls for cls in context.values() if hasattr(cls, "table_name") and cls.table_name == table_name]
202-
else:
203-
candidates = [target]
204-
205-
for candidate in candidates:
206-
instance = candidate() if isinstance(candidate, type) else candidate
207-
heading = getattr(instance, "_heading", None) or getattr(type(instance), "_heading", None)
208-
if heading is not None:
209-
heading._attributes = None
210-
heading._table_status = None
211-
212-
213189
def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
214190
"""
215191
Add the hidden ``_prov`` attribute to Entry (``dj.Manual``) tables that lack it.
@@ -250,6 +226,11 @@ def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
250226
a part table inherits its master's.
251227
- Rows already present keep ``NULL``. Provenance is recorded at insert and
252228
is never reconstructed after the fact.
229+
- **Takes effect on the next schema load.** A ``Heading`` is read from the
230+
database once and memoized, so a table already in use keeps its pre-ALTER
231+
attributes for the life of the process and its inserts go on recording
232+
nothing. This is a deploy-time operation: run it before the workers that
233+
will write through it, as with :func:`set_replica_identity`.
253234
"""
254235
import re
255236

@@ -282,7 +263,7 @@ def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
282263
raise DataJointError("Cannot add the provenance column: the target has no database.")
283264

284265
adapter = connection.adapter
285-
column_sql = provenance.column_definition(adapter)
266+
column_sql, _prov_comment = provenance.column_definition(adapter)
286267

287268
# `tables_modified` and `columns_added` count work actually done, as
288269
# set_replica_identity does; a dry run reports through `ddl` and `details`.
@@ -317,11 +298,6 @@ def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
317298
result["details"].append({"table": f"{database}.{table_name}", "status": "pending" if dry_run else "added"})
318299
if not dry_run:
319300
connection.query(ddl)
320-
# Invalidate the cached heading: without this the table keeps its
321-
# pre-ALTER attributes for the life of the process, `_prov` stays
322-
# invisible to the insert path, and every insert silently records
323-
# nothing until something reconnects.
324-
_refresh_heading(target, table_name)
325301
result["tables_modified"] += 1
326302
result["columns_added"] += 1
327303

‎src/datajoint/jobs.py‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,16 @@
2424
logger = logging.getLogger(__name__.split(".")[0])
2525

2626

27-
#: Job metadata columns, as (name, DataJoint core type, default clause, comment).
28-
#: Declared through the adapter's type system rather than as hand-written SQL so
29-
#: the backend mapping lives in the one place that owns it -- `datetime(3)`
30-
#: becomes `timestamp(3)` on PostgreSQL, which the hand-written form dropped.
31-
JOB_METADATA_SPEC = (
32-
("_job_start_time", "datetime(3)", "DEFAULT NULL", "when computation began"),
33-
("_job_duration", "float32", "DEFAULT NULL", "computation duration in seconds"),
34-
("_job_version", "varchar(64)", "DEFAULT ''", "code version"),
27+
#: Job metadata columns, in DataJoint definition notation. Parsed by the same
28+
#: machinery as any user attribute, so the backend type mapping and the `:type:`
29+
#: comment come from the one place that owns them.
30+
JOB_METADATA_DEFINITION = (
31+
"_job_start_time = null : datetime(3) # when computation began",
32+
"_job_duration = null : float32 # computation duration in seconds",
33+
'_job_version = "" : varchar(64) # code version',
3534
)
3635

3736

38-
def job_metadata_column_definitions(adapter) -> list[str]:
39-
"""Return the DDL fragments declaring the hidden job-metadata columns."""
40-
return [
41-
adapter.format_column_definition(
42-
name=name,
43-
sql_type=adapter.core_type_to_sql(core_type),
44-
nullable=True,
45-
default=default,
46-
comment=f":{core_type}:{comment}",
47-
)
48-
for name, core_type, default, comment in JOB_METADATA_SPEC
49-
]
50-
51-
5237
def _get_job_version(config=None) -> str:
5338
"""
5439
Get version string based on config settings.

‎src/datajoint/provenance.py‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,25 +89,23 @@ def _jsonable(value):
8989
return str(value)
9090

9191

92-
#: Recorded in the column comment so `heading` reads the declared core type back
93-
#: as ``original_type``, exactly as it does for a user-declared attribute.
94-
PROV_COMMENT = ":json:extrinsic provenance for a row that entered from outside"
92+
#: The attribute, in DataJoint definition notation -- parsed by the same
93+
#: machinery as any user attribute.
94+
PROV_DEFINITION = "_prov = null : json # extrinsic provenance for a row that entered from outside"
9595

9696

9797
def column_definition(adapter):
98-
"""Return the DDL fragment declaring ``_prov``, in DataJoint's type system.
98+
"""Return (DDL fragment, comment) declaring ``_prov``.
9999
100-
Built from ``core_type_to_sql("json")`` and ``format_column_definition``
101-
rather than hand-written per backend, so the json/jsonb choice stays in the
102-
one place that already owns it and a new adapter needs nothing added.
100+
Used by ``deploy.add_prov_column`` to ALTER an existing table; declaration
101+
appends :data:`PROV_DEFINITION` to the table definition instead.
103102
"""
104-
return adapter.format_column_definition(
105-
name=PROV_ATTRIBUTE,
106-
sql_type=adapter.core_type_to_sql("json"),
107-
nullable=True,
108-
default="DEFAULT NULL",
109-
comment=PROV_COMMENT,
103+
from .declare import compile_attribute
104+
105+
_name, sql, _store, comment = compile_attribute(
106+
PROV_DEFINITION, in_key=False, foreign_key_sql=[], context={}, adapter=adapter
110107
)
108+
return sql, comment
111109

112110

113111
def build_payload(connection, config=None):

‎tests/integration/test_entry_provenance.py‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,11 @@ class Legacy(dj.Manual):
205205
# idempotent
206206
assert add_prov_column(Legacy, dry_run=False)["columns_added"] == 0
207207

208-
# No manual _init_from_database(): add_prov_column invalidates the cached
209-
# heading, and the next access reloads it -- the same way the insert path
210-
# does. Reaching for _init_from_database() here is what previously hid
211-
# that inserts kept recording nothing until the process reconnected.
212-
heading = Legacy().heading
213-
heading.attributes # force the lazy reload, as _has_prov_attribute does
214-
assert provenance.PROV_ATTRIBUTE in heading._attributes
208+
# add_prov_column is a deploy-time operation and does not reach into the
209+
# in-process cache; the schema is reloaded before anything writes through
210+
# it. Reload here for the same reason a deployment would.
211+
Legacy().heading._init_from_database()
212+
assert provenance.PROV_ATTRIBUTE in Legacy().heading._attributes
215213

216214
# the pre-existing row keeps NULL; a new row carries a record
217215
Legacy.insert1({"legacy_id": 2, "note": "after"})

0 commit comments

Comments
 (0)