Skip to content

Commit 02d07d6

Browse files
ldanilekConvex, Inc.
ldanilek
authored and
Convex, Inc.
committed
simplify TableIdentifier (#27191)
remove the `id()` method which is making it harder to do refactors. GitOrigin-RevId: cf1e2f66cf92c82e76a0aaae3f4ff209acb7879c
1 parent 7e87abb commit 02d07d6

File tree

28 files changed

+117
-112
lines changed

28 files changed

+117
-112
lines changed

‎crates/application/src/export_worker.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,8 @@ mod tests {
899899
use value::{
900900
assert_obj,
901901
export::ValueFormat,
902+
DeveloperDocumentId,
902903
ResolvedDocumentId,
903-
TableIdentifier,
904904
TableNamespace,
905905
};
906906

@@ -1113,7 +1113,7 @@ mod tests {
11131113
let file1: ParsedDocument<FileStorageEntry> = tx
11141114
.get(ResolvedDocumentId::new(
11151115
storage_table_id.tablet_id,
1116-
storage_table_id.table_number.id(file1_id.internal_id()),
1116+
DeveloperDocumentId::new(storage_table_id.table_number, file1_id.internal_id()),
11171117
))
11181118
.await?
11191119
.unwrap()

‎crates/common/src/document.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -556,10 +556,7 @@ impl ResolvedDocument {
556556
pub fn from_database(tablet_id: TabletId, value: ConvexValue) -> anyhow::Result<Self> {
557557
let object: ConvexObject = value.try_into()?;
558558
let id = match object.get(&FieldName::from(ID_FIELD.clone())) {
559-
Some(ConvexValue::String(s)) => {
560-
let document_id = DeveloperDocumentId::decode(s)?;
561-
DeveloperDocumentId::new(*document_id.table(), document_id.internal_id())
562-
},
559+
Some(ConvexValue::String(s)) => DeveloperDocumentId::decode(s)?,
563560
_ => anyhow::bail!("Object {} missing _id field", object),
564561
};
565562
let creation_time = match object.get(&FieldName::from(CREATION_TIME_FIELD.clone())) {

‎crates/common/src/schemas/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,8 @@ pub const MAX_INDEXES_PER_TABLE: usize = 64;
6363
#[derive(derive_more::Display, Debug, Clone, PartialEq)]
6464
pub enum SchemaValidationError {
6565
#[display(
66-
fmt = "Document with ID \"{}\" in table \"{table_name}\" does not match the schema: \
67-
{validation_error}",
68-
"id.encode()"
66+
fmt = "Document with ID \"{id}\" in table \"{table_name}\" does not match the schema: \
67+
{validation_error}"
6968
)]
7069
ExistingDocument {
7170
validation_error: ValidationError,

‎crates/common/src/schemas/validator.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,10 +1034,8 @@ impl Display for FieldValidator {
10341034
#[derive(derive_more::Display, Debug, Clone, PartialEq)]
10351035
pub enum ValidationError {
10361036
#[display(
1037-
fmt = "Found ID \"{}\" from table `{}`, which does not match the table name in validator \
1038-
`v.id(\"{validator_table}\")`.{context}",
1039-
"id.encode()",
1040-
found_table_name
1037+
fmt = "Found ID \"{id}\" from table `{found_table_name}`, which does not match the table \
1038+
name in validator `v.id(\"{validator_table}\")`.{context}"
10411039
)]
10421040
TableNamesDoNotMatch {
10431041
id: DeveloperDocumentId,
@@ -1046,9 +1044,8 @@ pub enum ValidationError {
10461044
context: ValidationContext,
10471045
},
10481046
#[display(
1049-
fmt = "Found ID \"{}\" from a system table, which does not match the table name in \
1050-
validator `v.id(\"{validator_table}\")`.{context}",
1051-
"id.encode()"
1047+
fmt = "Found ID \"{id}\" from a system table, which does not match the table name in \
1048+
validator `v.id(\"{validator_table}\")`.{context}"
10521049
)]
10531050
SystemTableReference {
10541051
id: DeveloperDocumentId,

‎crates/common/src/testing/persistence_test_suite.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ use value::{
2525
val,
2626
ConvexObject,
2727
ConvexValue,
28+
DeveloperDocumentId,
2829
InternalDocumentId,
2930
ResolvedDocumentId,
30-
TableIdentifier,
3131
TableMapping,
3232
TabletId,
3333
};
@@ -701,12 +701,18 @@ pub async fn same_internal_id_multiple_tables<P: Persistence>(p: Arc<P>) -> anyh
701701
let table2_id = id_generator.user_table_id(&str::parse("table2")?);
702702

703703
let doc1 = ResolvedDocument::new(
704-
ResolvedDocumentId::new(table1_id.tablet_id, table1_id.table_number.id(internal_id)),
704+
ResolvedDocumentId::new(
705+
table1_id.tablet_id,
706+
DeveloperDocumentId::new(table1_id.table_number, internal_id),
707+
),
705708
CreationTime::ONE,
706709
assert_obj!("value" => 1),
707710
)?;
708711
let doc2 = ResolvedDocument::new(
709-
ResolvedDocumentId::new(table2_id.tablet_id, table2_id.table_number.id(internal_id)),
712+
ResolvedDocumentId::new(
713+
table2_id.tablet_id,
714+
DeveloperDocumentId::new(table2_id.table_number, internal_id),
715+
),
710716
CreationTime::ONE,
711717
assert_obj!("value" => 2),
712718
)?;

‎crates/common/src/testing/test_id_generator.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ impl TestIdGenerator {
176176
let table_metadata = TableMetadata::new(namespace, table_name.clone(), table_number);
177177
let id = ResolvedDocumentId::new(
178178
tables_table_id.tablet_id,
179-
tables_table_id.table_number.id(table_id.0),
179+
DeveloperDocumentId::new(tables_table_id.table_number, table_id.0),
180180
);
181181
let doc = ResolvedDocument::new(id, CreationTime::ONE, table_metadata.try_into()?)?;
182182
let index_update = DatabaseIndexUpdate {
@@ -197,7 +197,7 @@ impl TestIdGenerator {
197197
let table_id = self.user_table_id(table_name);
198198
ResolvedDocumentId::new(
199199
table_id.tablet_id,
200-
table_id.table_number.id(self.generate_internal()),
200+
DeveloperDocumentId::new(table_id.table_number, self.generate_internal()),
201201
)
202202
}
203203

@@ -206,12 +206,12 @@ impl TestIdGenerator {
206206
let table_id = self.system_table_id(table_name);
207207
ResolvedDocumentId::new(
208208
table_id.tablet_id,
209-
table_id.table_number.id(self.generate_internal()),
209+
DeveloperDocumentId::new(table_id.table_number, self.generate_internal()),
210210
)
211211
}
212212

213213
pub fn generate_virtual(&mut self, table_name: &TableName) -> DeveloperDocumentId {
214214
let table_num = self.generate_virtual_table(table_name);
215-
table_num.id(self.generate_internal())
215+
DeveloperDocumentId::new(table_num, self.generate_internal())
216216
}
217217
}

‎crates/database/benches/subscriptions.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ use search::{
5353
use serde::Deserialize;
5454
use value::{
5555
assert_obj,
56+
DeveloperDocumentId,
5657
FieldPath,
5758
InternalId,
5859
ResolvedDocumentId,
59-
TableIdentifier,
6060
TabletId,
6161
TabletIdAndTableNumber,
6262
};
@@ -121,8 +121,10 @@ fn load_datasets(
121121
m += d.text.len();
122122
n += 1;
123123
let internal_id = alloc_id();
124-
let id =
125-
ResolvedDocumentId::new(table_id.tablet_id, table_id.table_number.id(internal_id));
124+
let id = ResolvedDocumentId::new(
125+
table_id.tablet_id,
126+
DeveloperDocumentId::new(table_id.table_number, internal_id),
127+
);
126128
let tokenizer = convex_en();
127129
{
128130
let mut stream = tokenizer.token_stream(&d.text);

‎crates/database/src/bootstrap_model/components/mod.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ use common::{
3636
types::IndexName,
3737
};
3838
use value::{
39+
DeveloperDocumentId,
3940
FieldPath,
4041
InternalId,
4142
ResolvedDocumentId,
42-
TableIdentifier,
4343
TableName,
4444
TableNamespace,
4545
};
@@ -176,7 +176,7 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> {
176176
.id(&COMPONENTS_TABLE)?;
177177
Ok(ResolvedDocumentId::new(
178178
component_table.tablet_id,
179-
component_table.table_number.id(component_internal_id),
179+
DeveloperDocumentId::new(component_table.table_number, component_internal_id),
180180
))
181181
}
182182

@@ -191,9 +191,10 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> {
191191
.id(&COMPONENT_DEFINITIONS_TABLE)?;
192192
Ok(ResolvedDocumentId::new(
193193
component_definitions_table.tablet_id,
194-
component_definitions_table
195-
.table_number
196-
.id(component_definition_internal_id),
194+
DeveloperDocumentId::new(
195+
component_definitions_table.table_number,
196+
component_definition_internal_id,
197+
),
197198
))
198199
}
199200

‎crates/database/src/bootstrap_model/defaults.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use common::{
3030
};
3131
use maplit::btreemap;
3232
use value::{
33+
DeveloperDocumentId,
3334
ResolvedDocumentId,
34-
TableIdentifier,
3535
TableNamespace,
3636
TableNumber,
3737
TabletId,
@@ -172,14 +172,14 @@ impl BootstrapTableIds {
172172
pub fn table_resolved_doc_id(&self, table_id: TabletId) -> ResolvedDocumentId {
173173
ResolvedDocumentId::new(
174174
self.tables_id.tablet_id,
175-
self.tables_id.table_number.id(table_id.0),
175+
DeveloperDocumentId::new(self.tables_id.table_number, table_id.0),
176176
)
177177
}
178178

179179
pub fn index_resolved_doc_id(&self, index_id: IndexId) -> ResolvedDocumentId {
180180
ResolvedDocumentId::new(
181181
self.index_id.tablet_id,
182-
self.index_id.table_number.id(index_id),
182+
DeveloperDocumentId::new(self.index_id.table_number, index_id),
183183
)
184184
}
185185

‎crates/database/src/bootstrap_model/import_facing.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use value::{
1717
FieldName,
1818
ResolvedDocumentId,
1919
Size,
20-
TableIdentifier,
2120
TableMapping,
2221
TableName,
2322
TableNamespace,
@@ -100,7 +99,10 @@ impl<'a, RT: Runtime> ImportFacingModel<'a, RT> {
10099
} else {
101100
self.tx.id_generator.generate_internal()
102101
};
103-
let id = ResolvedDocumentId::new(table_id.tablet_id, table_id.table_number.id(internal_id));
102+
let id = ResolvedDocumentId::new(
103+
table_id.tablet_id,
104+
DeveloperDocumentId::new(table_id.table_number, internal_id),
105+
);
104106

105107
let creation_time_field = FieldName::from(CREATION_TIME_FIELD.clone());
106108
let creation_time = if let Some(ConvexValue::Float64(f)) = value.get(&creation_time_field) {

‎crates/database/src/bootstrap_model/system_metadata.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use common::{
55
};
66
use value::{
77
ConvexObject,
8+
DeveloperDocumentId,
89
InternalId,
910
ResolvedDocumentId,
10-
TableIdentifier,
1111
TableName,
1212
TableNamespace,
1313
TabletIdAndTableNumber,
@@ -63,9 +63,10 @@ impl<'a, RT: Runtime> SystemMetadataModel<'a, RT> {
6363
let table_id = self.lookup_table_id(table)?;
6464
let id = ResolvedDocumentId::new(
6565
table_id.tablet_id,
66-
table_id
67-
.table_number
68-
.id(self.tx.id_generator.generate_internal()),
66+
DeveloperDocumentId::new(
67+
table_id.table_number,
68+
self.tx.id_generator.generate_internal(),
69+
),
6970
);
7071
let creation_time = self.tx.next_creation_time.increment()?;
7172
let document = ResolvedDocument::new(id, creation_time, value)?;
@@ -88,8 +89,10 @@ impl<'a, RT: Runtime> SystemMetadataModel<'a, RT> {
8889
anyhow::bail!(unauthorized_error("insert_metadata"));
8990
}
9091
let table_id = self.lookup_table_id(table)?;
91-
let document_id =
92-
ResolvedDocumentId::new(table_id.tablet_id, table_id.table_number.id(internal_id));
92+
let document_id = ResolvedDocumentId::new(
93+
table_id.tablet_id,
94+
DeveloperDocumentId::new(table_id.table_number, internal_id),
95+
);
9396
let creation_time = self.tx.next_creation_time.increment()?;
9497
let document = ResolvedDocument::new(document_id, creation_time, value)?;
9598
self.tx.insert_document(document).await

‎crates/database/src/bootstrap_model/table.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,11 @@ use common::{
3737
TableName,
3838
TabletIndexName,
3939
},
40-
value::{
41-
TableIdentifier,
42-
TabletIdAndTableNumber,
43-
},
40+
value::TabletIdAndTableNumber,
4441
};
4542
use errors::ErrorMetadata;
4643
use value::{
44+
DeveloperDocumentId,
4745
FieldPath,
4846
ResolvedDocumentId,
4947
TableNamespace,
@@ -390,7 +388,7 @@ impl<'a, RT: Runtime> TableModel<'a, RT> {
390388
let tables_table_id = self.tables_table_id()?;
391389
let table_doc_id = ResolvedDocumentId::new(
392390
tables_table_id.tablet_id,
393-
tables_table_id.table_number.id(tablet_id.0),
391+
DeveloperDocumentId::new(tables_table_id.table_number, tablet_id.0),
394392
);
395393
SystemMetadataModel::new_global(self.tx)
396394
.replace(table_doc_id, table_metadata.try_into()?)

‎crates/database/src/bootstrap_model/user_facing.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use value::{
2727
DeveloperDocumentId,
2828
ResolvedDocumentId,
2929
Size,
30-
TableIdentifier,
3130
TableName,
3231
TableNamespace,
3332
};
@@ -187,7 +186,10 @@ impl<'a, RT: Runtime> UserFacingModel<'a, RT> {
187186
.namespace(self.namespace)
188187
.name_to_id_user_input()(table)?;
189188
let document = ResolvedDocument::new(
190-
ResolvedDocumentId::new(table_id.tablet_id, table_id.table_number.id(internal_id)),
189+
ResolvedDocumentId::new(
190+
table_id.tablet_id,
191+
DeveloperDocumentId::new(table_id.table_number, internal_id),
192+
),
191193
creation_time,
192194
value,
193195
)?;

‎crates/database/src/database.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ use common::{
9191
value::{
9292
ConvexObject,
9393
ResolvedDocumentId,
94-
TableIdentifier,
9594
TableMapping,
9695
TabletId,
9796
VirtualTableMapping,
@@ -1123,7 +1122,7 @@ impl<RT: Runtime> Database<RT> {
11231122
.id(table_name)?;
11241123
let document_id = ResolvedDocumentId::new(
11251124
tables_table_id.tablet_id,
1126-
tables_table_id.table_number.id(table_id.tablet_id.0),
1125+
DeveloperDocumentId::new(tables_table_id.table_number, table_id.tablet_id.0),
11271126
);
11281127
let metadata = TableMetadata::new(
11291128
TableNamespace::Global,

‎crates/database/src/index_worker.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ use maplit::{
9494
};
9595
use tracing::log;
9696
use value::{
97+
DeveloperDocumentId,
9798
InternalDocumentId,
98-
TableIdentifier,
9999
TableNamespace,
100100
};
101101

@@ -393,7 +393,7 @@ impl<RT: Runtime> IndexWorker<RT> {
393393
let index_doc = tx
394394
.get(ResolvedDocumentId::new(
395395
index_table_id.tablet_id,
396-
index_table_id.table_number.id(index_id),
396+
DeveloperDocumentId::new(index_table_id.table_number, index_id),
397397
))
398398
.await?
399399
.ok_or_else(|| anyhow::anyhow!("Index {index_id:?} no longer exists"))?;
@@ -432,7 +432,7 @@ impl<RT: Runtime> IndexWorker<RT> {
432432
let index_doc = tx
433433
.get(ResolvedDocumentId::new(
434434
index_table_id.tablet_id,
435-
index_table_id.table_number.id(index_id),
435+
DeveloperDocumentId::new(index_table_id.table_number, index_id),
436436
))
437437
.await?
438438
.ok_or_else(|| anyhow::anyhow!("Index {index_id:?} no longer exists"))?;
@@ -484,7 +484,7 @@ impl<RT: Runtime> IndexWorker<RT> {
484484
let index_table_id = tx.bootstrap_tables().index_id;
485485
let full_index_id = ResolvedDocumentId::new(
486486
index_table_id.tablet_id,
487-
index_table_id.table_number.id(index_id),
487+
DeveloperDocumentId::new(index_table_id.table_number, index_id),
488488
);
489489
let index_doc = tx
490490
.get(full_index_id)

‎crates/database/src/index_workers/search_flusher.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ use storage::Storage;
4848
use sync_types::Timestamp;
4949
use tempfile::TempDir;
5050
use value::{
51+
DeveloperDocumentId,
5152
ResolvedDocumentId,
52-
TableIdentifier,
5353
};
5454

5555
use crate::{
@@ -351,7 +351,10 @@ impl<RT: Runtime, T: SearchIndex + 'static> SearchFlusher<RT, T> {
351351
backfill_state.segments.clone(),
352352
MultipartBuildType::IncrementalComplete {
353353
cursor: cursor.map(|cursor| {
354-
ResolvedDocumentId::new(tablet_id, table_number.id(cursor))
354+
ResolvedDocumentId::new(
355+
tablet_id,
356+
DeveloperDocumentId::new(table_number, cursor),
357+
)
355358
}),
356359
backfill_snapshot_ts,
357360
},

0 commit comments

Comments
 (0)