Skip to content

Commit 46f4ad9

Browse files
fix(#1433): replace cryptic ParseException with clear DataJointError
User-defined hidden attributes (names starting with `_`) are intentionally not supported. The framework filters hidden columns out of every public API surface — fetch, dict restriction, insert, update1, describe — and populates platform-managed hidden columns (`_job_*`, `_singleton`) via raw SQL during the populate() lifecycle, not via the user-facing methods. Allowing users to declare hidden columns produces a feature with no public-API write path, no describe() round-trip, and silent dict- restriction filtering. The right fix for cases users reach for hidden attributes (e.g. an index-backing hash like `params_hash`) is a regular attribute. Add a pre-flight check in compile_attribute that detects a leading underscore and raises DataJointError with a clear message pointing to the alternative, instead of leaking pyparsing internals: Attribute name in line "_hidden: bool" starts with an underscore. Names with leading underscore are reserved for platform-managed columns (e.g. _job_start_time, _singleton). Use a regular attribute name; if you need to control visibility at the call site, use proj(). Platform code is unaffected: `_job_*` and `_singleton` are injected programmatically *after* parsing, so they bypass compile_attribute. Replaces 7 unit tests asserting "parser accepts _" with 4 asserting "compile_attribute rejects _ with helpful message" and "parser remains strict". Fixes #1433
1 parent fd8034c commit 46f4ad9

2 files changed

Lines changed: 34 additions & 19 deletions

File tree

src/datajoint/declare.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def build_attribute_parser() -> pp.ParserElement:
147147
"""
148148
quoted = pp.QuotedString('"') ^ pp.QuotedString("'")
149149
colon = pp.Literal(":").suppress()
150-
attribute_name = pp.Word(pp.srange("[_a-z]"), pp.srange("[a-z0-9_]")).set_results_name("name")
150+
attribute_name = pp.Word(pp.srange("[a-z]"), pp.srange("[a-z0-9_]")).set_results_name("name")
151151
data_type = (
152152
pp.Combine(pp.Word(pp.alphas) + pp.SkipTo("#", ignore=quoted))
153153
^ pp.QuotedString("<", end_quote_char=">", unquote_results=False)
@@ -855,6 +855,14 @@ def compile_attribute(
855855
DataJointError
856856
If syntax is invalid, primary key is nullable, or blob has invalid default.
857857
"""
858+
if line.lstrip().startswith("_"):
859+
raise DataJointError(
860+
f'Attribute name in line "{line}" starts with an underscore. '
861+
"Names with leading underscore are reserved for platform-managed "
862+
"columns (e.g. _job_start_time, _singleton). Use a regular "
863+
"attribute name; if you need to control visibility at the call "
864+
"site, use proj()."
865+
)
858866
try:
859867
match = attribute_parser.parse_string(line + "#", parse_all=True)
860868
except pp.ParseException as err:
Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,41 @@
1-
"""Unit tests for hidden attribute names (leading underscore) in table declarations.
1+
"""Unit tests for the leading-underscore guard in attribute declarations.
22
3-
Regression coverage for issue #1433: the declaration parser previously rejected
4-
attribute names starting with ``_``, even though hidden-attribute semantics
5-
(``is_hidden = name.startswith("_")``) were already implemented in ``Heading``.
3+
Regression coverage for issue #1433: declarations like ``_hidden: bool``
4+
previously failed with a cryptic ``pyparsing.ParseException``. The framework
5+
intentionally does not support user-defined hidden attributes — those names
6+
are reserved for platform-managed columns (e.g. ``_job_start_time``,
7+
``_singleton``) which DataJoint injects programmatically after parsing.
8+
9+
This test ensures the user gets a clear ``DataJointError`` pointing to the
10+
right alternative, not a parser-internals error.
611
"""
712

813
import pytest
914

10-
from datajoint.declare import attribute_parser
15+
from datajoint.declare import attribute_parser, compile_attribute
16+
from datajoint.errors import DataJointError
1117

1218

1319
@pytest.mark.parametrize(
1420
"line",
1521
[
1622
"_hidden: bool",
1723
"_params_hash: varchar(32)",
18-
"_job_start_time=null: datetime(3)",
19-
"_a: int",
24+
" _leading_whitespace: int32",
2025
],
2126
)
22-
def test_parser_accepts_leading_underscore(line):
23-
match = attribute_parser.parse_string(line + "#", parse_all=True)
24-
assert match["name"].startswith("_")
27+
def test_compile_attribute_rejects_leading_underscore(line):
28+
"""The leading-underscore guard fires before the parser, so adapter is unused."""
29+
with pytest.raises(DataJointError, match="reserved for platform-managed"):
30+
compile_attribute(line, in_key=False, foreign_key_sql=[], context={}, adapter=None)
31+
32+
33+
def test_parser_still_rejects_leading_underscore():
34+
"""Parser regex itself remains strict; the helpful error fires before the parser."""
35+
import pyparsing as pp
36+
37+
with pytest.raises(pp.ParseException):
38+
attribute_parser.parse_string("_hidden: bool#", parse_all=True)
2539

2640

2741
def test_parser_still_accepts_plain_names():
@@ -34,11 +48,4 @@ def test_parser_rejects_digit_start():
3448
import pyparsing as pp
3549

3650
with pytest.raises(pp.ParseException):
37-
attribute_parser.parse_string("1bad: int#", parse_all=True)
38-
39-
40-
def test_parser_extracts_hidden_name_for_is_hidden_dispatch():
41-
"""The parsed name is what Heading uses to set is_hidden via name.startswith('_')."""
42-
match = attribute_parser.parse_string("_secret: int#", parse_all=True)
43-
name = match["name"]
44-
assert name.startswith("_")
51+
attribute_parser.parse_string("1bad: int32#", parse_all=True)

0 commit comments

Comments
 (0)