Skip to content

Commit f99a7d6

Browse files
ldanilekConvex, Inc.
ldanilek
authored and
Convex, Inc.
committed
more cleanup of TabletIdAndTableNumber (#27172)
GitOrigin-RevId: a7b4aa2a692d702b2b22478f8b3ac7b77f27b8b5
1 parent 43fea72 commit f99a7d6

File tree

25 files changed

+92
-166
lines changed

25 files changed

+92
-166
lines changed

‎crates/application/src/schema_worker/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,18 @@ impl<RT: Runtime> SchemaWorker<RT> {
133133

134134
for table_name in tables_to_check {
135135
let table_iterator = self.database.table_iterator(ts, 1000, None);
136-
let table_id = table_mapping.id(table_name)?;
136+
let tablet_id = table_mapping.name_to_tablet()(table_name.clone())?;
137137
let stream = table_iterator.stream_documents_in_table(
138-
table_id.tablet_id,
139-
*by_id_indexes.get(&table_id.tablet_id).ok_or_else(|| {
140-
anyhow::anyhow!("Failed to find id index for table id {table_id}")
138+
tablet_id,
139+
*by_id_indexes.get(&tablet_id).ok_or_else(|| {
140+
anyhow::anyhow!("Failed to find id index for table id {tablet_id}")
141141
})?,
142142
None,
143143
);
144144

145145
pin_mut!(stream);
146146
while let Some((doc, _ts)) = stream.try_next().await? {
147-
let table_name = table_mapping.tablet_name(doc.table().tablet_id)?;
147+
let table_name = table_mapping.tablet_name(doc.id().tablet_id)?;
148148
log_document_validated();
149149
log_document_bytes(doc.size());
150150
if let Err(schema_error) = db_schema.check_existing_document(

‎crates/common/src/document.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ use value::{
5252
ResolvedDocumentId,
5353
TableNumber,
5454
TabletId,
55-
TabletIdAndTableNumber,
5655
MAX_DOCUMENT_NESTING,
5756
};
5857

@@ -617,13 +616,6 @@ impl ResolvedDocument {
617616
InternalDocumentId::new(self.tablet_id, self.id.internal_id())
618617
}
619618

620-
pub fn table(&self) -> TabletIdAndTableNumber {
621-
TabletIdAndTableNumber {
622-
tablet_id: self.tablet_id,
623-
table_number: *self.id.table(),
624-
}
625-
}
626-
627619
pub fn id(&self) -> ResolvedDocumentId {
628620
ResolvedDocumentId::new(self.tablet_id, self.id)
629621
}
@@ -745,12 +737,6 @@ impl PackedDocument {
745737
self.1
746738
}
747739

748-
/// Same behavior as ResolvedDocument::table but you don't have to fully
749-
/// unpack.
750-
pub fn tablet_id_and_number(&self) -> TabletIdAndTableNumber {
751-
self.id().tablet_id_and_number()
752-
}
753-
754740
pub fn value(&self) -> &PackedValue<ByteBuffer> {
755741
&self.0
756742
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ impl DatabaseSchema {
352352
virtual_table_mapping: &VirtualTableMapping,
353353
) -> Result<(), ValidationError> {
354354
if self.schema_validation
355-
&& let Ok(table_name) = table_mapping.tablet_name(doc.table().tablet_id)
355+
&& let Ok(table_name) = table_mapping.tablet_name(doc.id().tablet_id)
356356
&& let Some(document_schema) = self.schema_for_table(&table_name)
357357
{
358358
return document_schema.check_value(

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
261261

262262
test_load_documents_from_table(
263263
&p,
264-
doc1.table().tablet_id,
264+
doc1.id().tablet_id,
265265
TimestampRange::all(),
266266
Order::Asc,
267267
vec![
@@ -277,7 +277,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
277277

278278
test_load_documents_from_table(
279279
&p,
280-
doc2.table().tablet_id,
280+
doc2.id().tablet_id,
281281
TimestampRange::all(),
282282
Order::Asc,
283283
vec![
@@ -293,7 +293,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
293293

294294
test_load_documents_from_table(
295295
&p,
296-
doc1.table().tablet_id,
296+
doc1.id().tablet_id,
297297
TimestampRange::new(Timestamp::must(1)..)?,
298298
Order::Asc,
299299
vec![(Timestamp::must(1), doc1.id_with_table_id(), None)],
@@ -302,7 +302,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
302302

303303
test_load_documents_from_table(
304304
&p,
305-
doc2.table().tablet_id,
305+
doc2.id().tablet_id,
306306
TimestampRange::new(Timestamp::must(1)..)?,
307307
Order::Asc,
308308
vec![(Timestamp::must(1), doc2.id_with_table_id(), None)],
@@ -311,7 +311,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
311311

312312
test_load_documents_from_table(
313313
&p,
314-
doc1.table().tablet_id,
314+
doc1.id().tablet_id,
315315
TimestampRange::new(..Timestamp::must(1))?,
316316
Order::Asc,
317317
vec![(
@@ -324,7 +324,7 @@ pub async fn write_and_load_from_table<P: Persistence>(p: Arc<P>) -> anyhow::Res
324324

325325
test_load_documents_from_table(
326326
&p,
327-
doc2.table().tablet_id,
327+
doc2.id().tablet_id,
328328
TimestampRange::new(..Timestamp::must(1))?,
329329
Order::Asc,
330330
vec![(

‎crates/database/benches/subscriptions.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,13 @@ fn load_datasets(
160160
}
161161

162162
fn create_subscription_token(
163-
table_id: TabletIdAndTableNumber,
163+
tablet_id: TabletId,
164164
prefix: bool,
165165
max_distance: FuzzyDistance,
166166
token: String,
167167
) -> Token {
168-
let index_name: GenericIndexName<TabletId> = GenericIndexName::new(
169-
table_id.tablet_id,
170-
IndexDescriptor::from_str("index").unwrap(),
171-
)
172-
.unwrap();
168+
let index_name: GenericIndexName<TabletId> =
169+
GenericIndexName::new(tablet_id, IndexDescriptor::from_str("index").unwrap()).unwrap();
173170

174171
Token::text_search_token(
175172
index_name,
@@ -183,7 +180,7 @@ fn create_subscription_token(
183180
}
184181

185182
fn create_tokens(
186-
table_id: TabletIdAndTableNumber,
183+
tablet_id: TabletId,
187184
terms_by_frequency: &Vec<String>,
188185
prefix: bool,
189186
max_distance: FuzzyDistance,
@@ -199,20 +196,20 @@ fn create_tokens(
199196
.take(count)
200197
.map(|chunk| {
201198
let token = chunk.into_iter().next().unwrap();
202-
create_subscription_token(table_id, prefix, max_distance, token.clone())
199+
create_subscription_token(tablet_id, prefix, max_distance, token.clone())
203200
})
204201
.collect::<Vec<_>>()
205202
}
206203

207204
fn create_subscriptions(
208-
table_id: TabletIdAndTableNumber,
205+
tablet_id: TabletId,
209206
terms_by_frequency: &Vec<String>,
210207
prefix: bool,
211208
max_distance: FuzzyDistance,
212209
count: usize,
213210
) -> SubscriptionManager {
214211
let mut subscription_manager = SubscriptionManager::new_for_testing();
215-
let tokens = create_tokens(table_id, terms_by_frequency, prefix, max_distance, count);
212+
let tokens = create_tokens(tablet_id, terms_by_frequency, prefix, max_distance, count);
216213
for token in tokens {
217214
subscription_manager.subscribe(token).unwrap();
218215
}
@@ -229,7 +226,7 @@ fn bench_query(c: &mut Criterion) {
229226
for (prefix, max_distance) in prefix_and_max_distances() {
230227
for (dataset, (data, terms_by_frequency)) in &datasets {
231228
let subscription_manager = create_subscriptions(
232-
table_id,
229+
table_id.tablet_id,
233230
terms_by_frequency,
234231
prefix,
235232
max_distance,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ impl<'a, RT: Runtime> SchemaModel<'a, RT> {
172172
document: &ResolvedDocument,
173173
table_mapping_for_schema: &NamespacedTableMapping,
174174
) -> anyhow::Result<()> {
175-
let table_name = table_mapping_for_schema.tablet_name(document.table().tablet_id)?;
175+
let table_name = table_mapping_for_schema.tablet_name(document.id().tablet_id)?;
176176
if let Some((_id, active_schema)) = self.get_by_state(SchemaState::Active).await? {
177177
if let Err(schema_error) = active_schema.check_new_document(
178178
document,

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -484,28 +484,28 @@ impl<'a, RT: Runtime> TableModel<'a, RT> {
484484
let table_doc_id = SystemMetadataModel::new_global(self.tx)
485485
.insert_metadata(&TABLES_TABLE, table_metadata.try_into()?)
486486
.await?;
487-
let table_id = TabletIdAndTableNumber {
488-
tablet_id: TabletId(table_doc_id.internal_id()),
489-
table_number,
490-
};
487+
let tablet_id = TabletId(table_doc_id.internal_id());
491488

492489
// Add the system defined indexes for the newly created table. Since the newly
493490
// created table is empty, we can start these indexes as `Enabled`.
494491
let metadata = IndexMetadata::new_enabled(
495-
GenericIndexName::by_id(table_id.tablet_id),
492+
GenericIndexName::by_id(tablet_id),
496493
IndexedFields::by_id(),
497494
);
498495
SystemMetadataModel::new_global(self.tx)
499496
.insert_metadata(&INDEX_TABLE, metadata.try_into()?)
500497
.await?;
501498
let metadata = IndexMetadata::new_enabled(
502-
GenericIndexName::by_creation_time(table_id.tablet_id),
499+
GenericIndexName::by_creation_time(tablet_id),
503500
IndexedFields::creation_time(),
504501
);
505502
SystemMetadataModel::new_global(self.tx)
506503
.insert_metadata(&INDEX_TABLE, metadata.try_into()?)
507504
.await?;
508-
Ok(table_id)
505+
Ok(TabletIdAndTableNumber {
506+
tablet_id,
507+
table_number,
508+
})
509509
}
510510
}
511511

‎crates/database/src/committer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ impl<RT: Runtime> Committer<RT> {
773773
} = validated_write;
774774
if let Some(document) = document {
775775
let document_write_size = document_id.size() + document.size();
776-
if let Ok(table_name) = table_mapping.tablet_name(document.table().tablet_id) {
776+
if let Ok(table_name) = table_mapping.tablet_name(document.id().tablet_id) {
777777
// Database bandwidth for document writes
778778
if *doc_in_vector_index == DocInVectorIndex::Absent {
779779
usage_tracker.track_database_ingress_size(

‎crates/database/src/database.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,10 +1043,9 @@ impl<RT: Runtime> Database<RT> {
10431043
let index_tablet_id = snapshot.index_registry.index_table();
10441044
let index_by_id = snapshot
10451045
.index_registry
1046-
.must_get_by_id(index_tablet_id.tablet_id)?
1046+
.must_get_by_id(index_tablet_id)?
10471047
.id();
1048-
let stream =
1049-
table_iterator.stream_documents_in_table(index_tablet_id.tablet_id, index_by_id, None);
1048+
let stream = table_iterator.stream_documents_in_table(index_tablet_id, index_by_id, None);
10501049
pin_mut!(stream);
10511050
let mut by_id_indexes = BTreeMap::new();
10521051
while let Some((index_doc, _)) = stream.try_next().await? {
@@ -1191,7 +1190,7 @@ impl<RT: Runtime> Database<RT> {
11911190
// Build the index metadata from the index documents.
11921191
let index_documents = document_writes
11931192
.iter()
1194-
.filter(|(id, _)| id.tablet_id_and_number() == index_table_id)
1193+
.filter(|(id, _)| id.tablet_id == index_table_id.tablet_id)
11951194
.map(|(id, doc)| (*id, (ts, doc.clone())))
11961195
.collect::<BTreeMap<_, _>>();
11971196
let mut index_registry = IndexRegistry::bootstrap(

‎crates/database/src/retention.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,7 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
257257
let snapshot = snapshot_reader.lock().latest_snapshot();
258258
let index_registry = snapshot.index_registry;
259259
let meta_index_id = index_registry
260-
.enabled_index_metadata(&TabletIndexName::by_id(
261-
index_registry.index_table().tablet_id,
262-
))
260+
.enabled_index_metadata(&TabletIndexName::by_id(index_registry.index_table()))
263261
.expect("meta index id must exist")
264262
.id()
265263
.internal_id();
@@ -277,7 +275,7 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
277275
let reader = reader.read_snapshot(snapshot_ts)?;
278276
let mut meta_index_scan = reader.index_scan(
279277
meta_index_id,
280-
index_registry.index_table().tablet_id,
278+
index_registry.index_table(),
281279
&Interval::all(),
282280
Order::Asc,
283281
usize::MAX,

‎crates/database/src/table_summary.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ mod tests {
782782
let table_id = tx
783783
.table_mapping()
784784
.namespace(TableNamespace::test_user())
785-
.id(table_name)?;
785+
.name_to_tablet()(table_name.clone())?;
786786
let summary = expected.entry(table_id).or_insert_with(TableSummary::empty);
787787
let inserted = inserted.value().0.clone();
788788
*summary = summary.insert(&inserted);
@@ -804,7 +804,7 @@ mod tests {
804804
let table_id = table_mapping
805805
.namespace(TableNamespace::test_user())
806806
.id(table_name)?;
807-
let expected = expected.get(&table_id).unwrap();
807+
let expected = expected.get(&table_id.tablet_id).unwrap();
808808
assert_eq!(expected, computed.tables.get(&table_id.tablet_id).unwrap());
809809
}
810810
}

‎crates/database/src/tests/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ use value::{
8989
TableIdentifier,
9090
TableMapping,
9191
TableNamespace,
92+
TabletIdAndTableNumber,
9293
};
9394

9495
use crate::{
@@ -474,7 +475,10 @@ async fn test_id_reuse_across_transactions(rt: TestRuntime) -> anyhow::Result<()
474475
let table_mapping_for_schema = tx.table_mapping().clone();
475476
ImportFacingModel::new(&mut tx)
476477
.insert(
477-
document.table(),
478+
TabletIdAndTableNumber {
479+
tablet_id: document.id().tablet_id,
480+
table_number: *document.id().developer_id.table(),
481+
},
478482
&"table".parse()?,
479483
assert_obj!("_id" => id_v6),
480484
&table_mapping_for_schema,

‎crates/database/src/tests/streaming_export_tests.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,19 @@ async fn test_document_deltas(rt: TestRuntime) -> anyhow::Result<()> {
5353
(
5454
ts1,
5555
doc1sort.developer_id(),
56-
table_mapping.tablet_name(doc1sort.table().tablet_id)?,
56+
table_mapping.tablet_name(doc1sort.id().tablet_id)?,
5757
Some(doc1sort.clone())
5858
),
5959
(
6060
ts1,
6161
doc2sort.developer_id(),
62-
table_mapping.tablet_name(doc2sort.table().tablet_id)?,
62+
table_mapping.tablet_name(doc2sort.id().tablet_id)?,
6363
Some(doc2sort.clone())
6464
),
6565
(
6666
ts2,
6767
doc3.developer_id(),
68-
table_mapping.tablet_name(doc3.table().tablet_id)?,
68+
table_mapping.tablet_name(doc3.id().tablet_id)?,
6969
Some(doc3.clone())
7070
),
7171
],
@@ -83,7 +83,7 @@ async fn test_document_deltas(rt: TestRuntime) -> anyhow::Result<()> {
8383
deltas: vec![(
8484
ts2,
8585
doc3.developer_id(),
86-
table_mapping.tablet_name(doc3.table().tablet_id)?,
86+
table_mapping.tablet_name(doc3.id().tablet_id)?,
8787
Some(doc3.clone())
8888
)],
8989
cursor: ts2,
@@ -100,7 +100,7 @@ async fn test_document_deltas(rt: TestRuntime) -> anyhow::Result<()> {
100100
deltas: vec![(
101101
ts1,
102102
doc1.developer_id(),
103-
table_mapping.tablet_name(doc1.table().tablet_id)?,
103+
table_mapping.tablet_name(doc1.id().tablet_id)?,
104104
Some(doc1.clone())
105105
)],
106106
cursor: ts2,
@@ -120,13 +120,13 @@ async fn test_document_deltas(rt: TestRuntime) -> anyhow::Result<()> {
120120
(
121121
ts1,
122122
doc1sort.developer_id(),
123-
table_mapping.tablet_name(doc1sort.table().tablet_id)?,
123+
table_mapping.tablet_name(doc1sort.id().tablet_id)?,
124124
Some(doc1sort.clone())
125125
),
126126
(
127127
ts1,
128128
doc2sort.developer_id(),
129-
table_mapping.tablet_name(doc2sort.table().tablet_id)?,
129+
table_mapping.tablet_name(doc2sort.id().tablet_id)?,
130130
Some(doc2sort.clone())
131131
),
132132
],
@@ -208,7 +208,7 @@ async fn document_deltas_should_not_ignore_rows_from_tables_that_were_not_delete
208208
deltas: vec![(
209209
ts_insert,
210210
remaining_doc.developer_id(),
211-
table_mapping.tablet_name(remaining_doc.table().tablet_id)?,
211+
table_mapping.tablet_name(remaining_doc.id().tablet_id)?,
212212
Some(remaining_doc.clone())
213213
),],
214214
cursor: ts_latest,

0 commit comments

Comments
 (0)