refactor(provenance): inline the tier test, carry _prov through INSERT ... SELECT

Three follow-ups from review of #1555.

The Entry-table test is now inlined at its two call sites rather than wrapped in
`provenance.is_entry_table`. The import stays deferred inside the function:
user_tables imports table, which imports declare, so a module-scope import is a
cycle -- which is what the wrapper had been hiding.

`insert(QueryExpression)` builds INSERT ... SELECT and returned before the
provenance was attached, so copied rows landed with NULL. It now carries the
source's `_prov` across. A copied row did not originate in the destination, so
the source's record is the true one; re-stamping it here would claim an origin
that is not where the data came from.

`heading.as_sql` resolves an explicitly named hidden attribute against the full
attribute set. Default field lists are built from `attributes` and still never
contain hidden names, so nothing else changes.

681 passed, 14 skipped.

Author: dimitri-yatsenko

src/datajoint/declare.py

@@ -12,7 +12,6 @@
 
 import pyparsing as pp
 
-from . import provenance
 from .codecs import lookup_codec
 from .condition import translate_attribute
 from .errors import DataJointError
@@ -526,7 +525,14 @@ def declare(
     # from outside the pipeline.  Computed and Imported tables have no use for
     # it -- their provenance is entailed by the foreign-key graph -- and a part
     # inherits its master's.
-    if config.provenance.capture and provenance.is_entry_table(table_name):
+    # Matched against the Manual tier itself, not by excluding the other tiers'
+    # prefixes: enumerating exclusions makes every tier added later an Entry
+    # table by default, which is how job tables (`~`) first acquired the slot.
+    # Imported here rather than at module scope: user_tables imports table,
+    # which imports this module.
+    from .user_tables import Manual
+
+    if config.provenance.capture and re.fullmatch(Manual.tier_regexp, table_name):
         attribute_sql.extend(adapter.provenance_columns())
 
     if not primary_key:

src/datajoint/deploy.py

@@ -227,9 +227,12 @@ def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
     - Rows already present keep ``NULL``.  Provenance is recorded at insert and
       is never reconstructed after the fact.
     """
+    import re
+
     from . import provenance
     from .schemas import _Schema
     from .table import Table
+    from .user_tables import Manual
 
     if isinstance(target, _Schema):
         connection = target.connection
@@ -266,7 +269,7 @@ def add_prov_column(target: "TargetType", dry_run: bool = True) -> dict:
     }
 
     for table_name in table_names:
-        if not provenance.is_entry_table(table_name):
+        if not re.fullmatch(Manual.tier_regexp, table_name):
             continue
         result["tables_analyzed"] += 1
 

src/datajoint/heading.py

@@ -397,7 +397,11 @@ def quote(name):
             return adapter.quote_identifier(name) if adapter else f'"{name}"'
 
         def render_field(name):
-            attr = self.attributes[name]
+            # `attributes` hides underscore-prefixed names, so a caller that asks
+            # for one by name -- copying `_prov` through an INSERT ... SELECT --
+            # falls back to the full set. Default field lists are unaffected:
+            # they are built from `attributes` and never contain hidden names.
+            attr = self.attributes.get(name) or self._attributes[name]
             if attr.attribute_expression is None:
                 return quote(name)
             else:

src/datajoint/provenance.py

@@ -29,7 +29,6 @@
 import contextvars
 import datetime
 import json
-import re
 from typing import Any
 
 #: Name of the hidden attribute.  Hidden attributes are excluded from
@@ -90,23 +89,6 @@ def _jsonable(value):
     return str(value)
 
 
-def is_entry_table(table_name):
-    """Whether a stripped table name denotes an Entry (``dj.Manual``) table.
-
-    Matched against ``Manual.tier_regexp``, the definition the rest of the
-    library uses, rather than by excluding the prefixes of the other tiers.
-    Enumerating prefixes means every tier added later is an Entry table until
-    someone remembers this function -- which is how job tables (``~``) first
-    acquired the slot.
-
-    A part table carries its master's name and ``__`` before its own, so it
-    fails the match and inherits its master's provenance, which is what we want.
-    """
-    from .user_tables import Manual
-
-    return re.fullmatch(Manual.tier_regexp, table_name) is not None
-
-
 def build_payload(connection, config=None):
     """Assemble the provenance record for rows inserted on this connection.
 

src/datajoint/table.py

@@ -878,6 +878,12 @@ def insert(
                 except StopIteration:
                     pass
             fields = list(name for name in rows.heading if name in self.heading)
+            # Carry provenance across rather than leaving the copies NULL. A row
+            # copied from another table did not originate here, so the source's
+            # record is the true one; re-stamping it with this moment would claim
+            # an origin that is not where the data came from.
+            if self._has_prov_attribute() and provenance.PROV_ATTRIBUTE in (rows.heading._attributes or {}):
+                fields.append(provenance.PROV_ATTRIBUTE)
             quoted_fields = ",".join(self.adapter.quote_identifier(f) for f in fields)
 
             # Duplicate handling (backend-agnostic)

tests/integration/test_entry_provenance.py

@@ -246,3 +246,26 @@ def test_job_tables_do_not_get_prov(schema_prov):
             ).fetchall()
         }
         assert provenance.PROV_ATTRIBUTE not in columns, f"{name} carries {provenance.PROV_ATTRIBUTE}"
+
+
+def test_insert_from_query_carries_provenance_across(schema_prov):
+    """`insert(QueryExpression)` builds INSERT ... SELECT and used to leave NULL.
+
+    A copied row did not originate in the destination, so the source's record is
+    the true one; re-stamping it here would claim an origin that is not where
+    the data came from.
+    """
+    schema, t = schema_prov
+    t["Subject"].insert1({"subject_id": 40, "species": "mouse"})
+    (original,) = _raw_prov(t["Subject"]() & "subject_id = 40")
+    assert original is not None
+
+    class SubjectCopy(dj.Manual):
+        definition = t["Subject"].definition
+
+    schema(SubjectCopy)
+    SubjectCopy.insert(t["Subject"]() & "subject_id = 40")
+
+    (copied,) = _raw_prov(SubjectCopy())
+    assert copied is not None, "copied row lost its provenance"
+    assert copied == original, "copied row was re-stamped instead of carrying its origin"

tests/unit/test_provenance.py

@@ -5,6 +5,7 @@
 """
 
 import json
+import re
 
 import pytest
 
@@ -46,8 +47,14 @@ def config():
     ],
 )
 def test_is_entry_table(table_name, expected):
-    """Only Entry tables get the slot; parts inherit their master's."""
-    assert provenance.is_entry_table(table_name) is expected
+    """Only Entry tables get the slot; parts inherit their master's.
+
+    Exercises the predicate `declare` and `deploy` use: a match against the
+    Manual tier itself, rather than a list of prefixes to exclude.
+    """
+    from datajoint.user_tables import Manual
+
+    assert bool(re.fullmatch(Manual.tier_regexp, table_name)) is expected
 
 
 def test_payload_is_none_without_anything_to_say(config):
@@ -138,18 +145,19 @@ def test_entry_test_follows_the_tier_definition_not_a_prefix_list():
     `Manual.tier_regexp` inverts that: a name is an Entry table only if the
     library says it is.
     """
-    import re
-
     from datajoint.user_tables import Computed, Imported, Lookup, Manual, Part
 
-    assert provenance.is_entry_table("subject")
+    def grants_prov(name):
+        return re.fullmatch(Manual.tier_regexp, name) is not None
+
+    assert grants_prov("subject")
     for tier in (Lookup, Imported, Computed, Part):
         sample = {Lookup: "#param", Imported: "_ingest", Computed: "__analysis", Part: "subject__detail"}[tier]
         assert re.fullmatch(tier.tier_regexp, sample), f"{sample} is not a {tier.__name__}"
-        assert not provenance.is_entry_table(sample)
+        assert not grants_prov(sample)
     # The job prefix belongs to no user tier at all, which is how it slipped through.
     assert not any(re.fullmatch(t.tier_regexp, "~~analysis") for t in (Manual, Lookup, Imported, Computed, Part))
-    assert not provenance.is_entry_table("~~analysis")
+    assert not grants_prov("~~analysis")
 
 
 def test_serialize_survives_a_deployment_supplied_source(config):