Skip to content

Commit e84ff9a

Browse files
committed
fix(control): stop duplicating explicit id in DESCRIBE
DESCRIBE on a strict collection created with an explicit id ... PRIMARY KEY listed the id field twice — once from the unconditional synthetic id row and once from the declared field list — with contradictory nullability. The synthetic id row is now emitted only when the collection doesn't already declare one, and each declared field's nullability is derived from its PRIMARY KEY / NOT NULL type modifiers instead of being hardcoded nullable. Fixes #173.
1 parent 67ed322 commit e84ff9a

2 files changed

Lines changed: 64 additions & 4 deletions

File tree

nodedb/src/control/server/shared/ddl/neutral/collection/describe.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,28 @@ pub fn describe_collection(
7272

7373
let mut rows = Vec::new();
7474

75-
// Always has an 'id' field.
76-
push_describe_row(&mut rows, "id", "TEXT", "false");
77-
75+
// Synthesize the implicit 'id' field only when the collection does not
76+
// already declare one — a strict collection created with an explicit
77+
// `id ... PRIMARY KEY` stores `id` in `coll.fields`, so emitting the
78+
// synthetic row too would list `id` twice with contradictory nullability.
79+
let declares_id = coll
80+
.fields
81+
.iter()
82+
.any(|(name, _)| name.eq_ignore_ascii_case("id"));
83+
if !declares_id {
84+
push_describe_row(&mut rows, "id", "TEXT", "false");
85+
}
7886
if coll.fields.is_empty() {
7987
push_describe_row(&mut rows, "document", "JSON", "true");
8088
} else {
8189
for (field_name, field_type) in &coll.fields {
82-
push_describe_row(&mut rows, field_name, field_type, "true");
90+
let upper = field_type.to_uppercase();
91+
let nullable = if upper.contains("PRIMARY KEY") || upper.contains("NOT NULL") {
92+
"false"
93+
} else {
94+
"true"
95+
};
96+
push_describe_row(&mut rows, field_name, field_type, nullable);
8397
}
8498
}
8599

nodedb/tests/database_scoped_ddl_introspection.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,52 @@ async fn describe_collection_finds_it_in_non_default_database() {
8484
);
8585
}
8686

87+
/// Regression for issue #173: `DESCRIBE` on a strict collection created with an
88+
/// explicit `id ... PRIMARY KEY` listed the `id` field twice — once from the
89+
/// unconditional synthetic `id` row and once from iterating the declared
90+
/// `coll.fields` — with contradictory nullability. The synthetic row is now
91+
/// emitted only when the collection does not already declare an `id`, and each
92+
/// declared field's nullability is derived from its type modifiers.
93+
#[tokio::test]
94+
async fn describe_strict_does_not_duplicate_explicit_id() {
95+
let server = TestServer::start().await;
96+
97+
query_ok(
98+
&server,
99+
"CREATE COLLECTION desc_repro (id TEXT PRIMARY KEY, label TEXT) \
100+
WITH (engine='document_strict')",
101+
)
102+
.await;
103+
104+
let describe_rows = server
105+
.client
106+
.simple_query("DESCRIBE desc_repro")
107+
.await
108+
.unwrap_or_else(|e| panic!("DESCRIBE desc_repro must succeed: {e}"));
109+
110+
let id_rows: Vec<&tokio_postgres::SimpleQueryRow> = describe_rows
111+
.iter()
112+
.filter_map(|m| match m {
113+
tokio_postgres::SimpleQueryMessage::Row(row) if row.get("field") == Some("id") => {
114+
Some(row)
115+
}
116+
_ => None,
117+
})
118+
.collect();
119+
120+
assert_eq!(
121+
id_rows.len(),
122+
1,
123+
"DESCRIBE must list the explicit `id` PK field exactly once (pre-fix: twice), \
124+
got: {describe_rows:?}"
125+
);
126+
assert_eq!(
127+
id_rows[0].get("nullable"),
128+
Some("false"),
129+
"the `id` PRIMARY KEY field must be non-nullable, got: {describe_rows:?}"
130+
);
131+
}
132+
87133
/// `SHOW COLLECTIONS` in a non-default database must list collections
88134
/// created in that database.
89135
///

0 commit comments

Comments
 (0)