Skip to content

Commit c4534b0

Browse files
mgattozzimgattozzi
authored
feat: move Table Id/Name mapping into DB Schema (#25436)
1 parent bd20f80 commit c4534b0

15 files changed

+116
-212
lines changed

‎influxdb3_catalog/src/catalog.rs

Lines changed: 54 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,6 @@ impl Catalog {
167167
Ok(db)
168168
}
169169

170-
pub fn add_table_to_lookup(&self, db_id: DbId, table_id: TableId, name: Arc<str>) {
171-
self.inner
172-
.write()
173-
.table_map
174-
.entry(db_id)
175-
.or_default()
176-
.insert(table_id, name);
177-
}
178-
179170
pub fn db_name_to_id(&self, db_name: Arc<str>) -> Option<DbId> {
180171
self.inner.read().db_map.get_by_right(&db_name).copied()
181172
}
@@ -184,23 +175,6 @@ impl Catalog {
184175
self.inner.read().db_map.get_by_left(&db_id).map(Arc::clone)
185176
}
186177

187-
pub fn table_name_to_id(&self, db_id: DbId, table_name: Arc<str>) -> Option<TableId> {
188-
self.inner
189-
.read()
190-
.table_map
191-
.get(&db_id)
192-
.and_then(|map| map.get_by_right(&table_name).copied())
193-
}
194-
195-
pub fn table_id_to_name(&self, db_id: DbId, table_id: TableId) -> Option<Arc<str>> {
196-
self.inner
197-
.read()
198-
.table_map
199-
.get(&db_id)
200-
.and_then(|map| map.get_by_left(&table_id))
201-
.map(Arc::clone)
202-
}
203-
204178
pub fn db_schema(&self, id: &DbId) -> Option<Arc<DatabaseSchema>> {
205179
self.inner.read().databases.get(id).cloned()
206180
}
@@ -267,19 +241,6 @@ impl Catalog {
267241

268242
pub fn insert_database(&mut self, db: DatabaseSchema) {
269243
let mut inner = self.inner.write();
270-
for (table_id, table_def) in db.tables.iter() {
271-
inner
272-
.table_map
273-
.entry(db.id)
274-
.and_modify(|map: &mut BiHashMap<TableId, Arc<str>>| {
275-
map.insert(*table_id, Arc::clone(&table_def.table_name));
276-
})
277-
.or_insert_with(|| {
278-
let mut map = BiHashMap::new();
279-
map.insert(*table_id, Arc::clone(&table_def.table_name));
280-
map
281-
});
282-
}
283244
inner.db_map.insert(db.id, Arc::clone(&db.name));
284245
inner.databases.insert(db.id, Arc::new(db));
285246
inner.sequence = inner.sequence.next();
@@ -321,8 +282,6 @@ pub struct InnerCatalog {
321282
updated: bool,
322283
#[serde_as(as = "DbMapAsArray")]
323284
db_map: BiHashMap<DbId, Arc<str>>,
324-
#[serde_as(as = "TableMapAsArray")]
325-
pub table_map: HashMap<DbId, BiHashMap<TableId, Arc<str>>>,
326285
}
327286

328287
serde_with::serde_conv!(
@@ -351,45 +310,33 @@ struct DbMap {
351310
name: Arc<str>,
352311
}
353312

313+
#[derive(Debug, Serialize, Deserialize)]
314+
struct TableMap {
315+
table_id: TableId,
316+
name: Arc<str>,
317+
}
318+
354319
serde_with::serde_conv!(
355320
TableMapAsArray,
356-
HashMap<DbId, BiHashMap<TableId, Arc<str>>>,
357-
|map: &HashMap<DbId, BiHashMap<TableId, Arc<str>>>| {
358-
map.iter().fold(Vec::new(), |mut acc, (db_id, table_map)| {
359-
for (table_id, name) in table_map.iter() {
360-
acc.push(TableMap {
361-
db_id: *db_id,
362-
table_id: *table_id,
363-
name: Arc::clone(&name)
364-
});
365-
}
321+
BiHashMap<TableId, Arc<str>>,
322+
|map: &BiHashMap<TableId, Arc<str>>| {
323+
map.iter().fold(Vec::new(), |mut acc, (table_id, name)| {
324+
acc.push(TableMap {
325+
table_id: *table_id,
326+
name: Arc::clone(&name)
327+
});
366328
acc
367329
})
368330
},
369331
|vec: Vec<TableMap>| -> Result<_, std::convert::Infallible> {
370-
let mut map = HashMap::new();
332+
let mut map = BiHashMap::new();
371333
for item in vec {
372-
map.entry(item.db_id)
373-
.and_modify(|entry: &mut BiHashMap<TableId, Arc<str>>| {
374-
entry.insert(item.table_id, Arc::clone(&item.name));
375-
})
376-
.or_insert_with(||{
377-
let mut inner_map = BiHashMap::new();
378-
inner_map.insert(item.table_id, Arc::clone(&item.name));
379-
inner_map
380-
});
334+
map.insert(item.table_id, item.name);
381335
}
382336
Ok(map)
383337
}
384338
);
385339

386-
#[derive(Debug, Serialize, Deserialize)]
387-
struct TableMap {
388-
db_id: DbId,
389-
table_id: TableId,
390-
name: Arc<str>,
391-
}
392-
393340
serde_with::serde_conv!(
394341
DatabasesAsArray,
395342
HashMap<DbId, Arc<DatabaseSchema>>,
@@ -406,17 +353,20 @@ serde_with::serde_conv!(
406353
|vec: Vec<DatabasesSerialized>| -> Result<_, String> {
407354
vec.into_iter().fold(Ok(HashMap::new()), |acc, db| {
408355
let mut acc = acc?;
356+
let mut table_map = BiHashMap::new();
409357
if let Some(_) = acc.insert(db.id, Arc::new(DatabaseSchema {
410358
id: db.id,
411359
name: Arc::clone(&db.name),
412360
tables: db.tables.into_iter().fold(Ok(BTreeMap::new()), |acc, table| {
413361
let mut acc = acc?;
414362
let table_name = Arc::clone(&table.table_name);
363+
table_map.insert(table.table_id, Arc::clone(&table_name));
415364
if let Some(_) = acc.insert(table.table_id, table) {
416365
return Err(format!("found duplicate table: {}", table_name));
417366
}
418367
Ok(acc)
419-
})?
368+
})?,
369+
table_map
420370
})) {
421371
return Err(format!("found duplicate db: {}", db.name));
422372
}
@@ -441,7 +391,6 @@ impl InnerCatalog {
441391
instance_id,
442392
updated: false,
443393
db_map: BiHashMap::new(),
444-
table_map: HashMap::new(),
445394
}
446395
}
447396

@@ -471,18 +420,6 @@ impl InnerCatalog {
471420
self.sequence = self.sequence.next();
472421
self.updated = true;
473422
self.db_map.insert(new_db.id, Arc::clone(&new_db.name));
474-
for (table_id, table_def) in new_db.tables.iter() {
475-
self.table_map
476-
.entry(new_db.id)
477-
.and_modify(|map| {
478-
map.insert(*table_id, Arc::clone(&table_def.table_name));
479-
})
480-
.or_insert_with(|| {
481-
let mut map = BiHashMap::new();
482-
map.insert(*table_id, Arc::clone(&table_def.table_name));
483-
map
484-
});
485-
}
486423
}
487424
} else {
488425
if self.databases.len() >= Catalog::NUM_DBS_LIMIT {
@@ -499,18 +436,6 @@ impl InnerCatalog {
499436
self.sequence = self.sequence.next();
500437
self.updated = true;
501438
self.db_map.insert(new_db.id, Arc::clone(&new_db.name));
502-
for (table_id, table_def) in new_db.tables.iter() {
503-
self.table_map
504-
.entry(new_db.id)
505-
.and_modify(|map| {
506-
map.insert(*table_id, Arc::clone(&table_def.table_name));
507-
})
508-
.or_insert_with(|| {
509-
let mut map = BiHashMap::new();
510-
map.insert(*table_id, Arc::clone(&table_def.table_name));
511-
map
512-
});
513-
}
514439
}
515440

516441
Ok(())
@@ -532,6 +457,8 @@ pub struct DatabaseSchema {
532457
pub name: Arc<str>,
533458
/// The database is a map of tables
534459
pub tables: BTreeMap<TableId, TableDefinition>,
460+
#[serde_as(as = "TableMapAsArray")]
461+
pub table_map: BiHashMap<TableId, Arc<str>>,
535462
}
536463

537464
impl DatabaseSchema {
@@ -540,6 +467,7 @@ impl DatabaseSchema {
540467
id,
541468
name,
542469
tables: BTreeMap::new(),
470+
table_map: BiHashMap::new(),
543471
}
544472
}
545473

@@ -636,10 +564,17 @@ impl DatabaseSchema {
636564
}
637565
}
638566

567+
// With the final list of updated/new tables update the current mapping
568+
let new_table_maps = updated_or_new_tables
569+
.iter()
570+
.map(|(table_id, table_def)| (*table_id, Arc::clone(&table_def.table_name)))
571+
.collect();
572+
639573
Ok(Some(Self {
640574
id: self.id,
641575
name: Arc::clone(&self.name),
642576
tables: updated_or_new_tables,
577+
table_map: new_table_maps,
643578
}))
644579
}
645580
}
@@ -681,6 +616,14 @@ impl DatabaseSchema {
681616
pub fn tables(&self) -> impl Iterator<Item = &TableDefinition> {
682617
self.tables.values()
683618
}
619+
620+
pub fn table_name_to_id(&self, table_name: Arc<str>) -> Option<TableId> {
621+
self.table_map.get_by_right(&table_name).copied()
622+
}
623+
624+
pub fn table_id_to_name(&self, table_id: TableId) -> Option<Arc<str>> {
625+
self.table_map.get_by_left(&table_id).map(Arc::clone)
626+
}
684627
}
685628

686629
#[derive(Debug, Eq, PartialEq, Clone)]
@@ -960,6 +903,12 @@ mod tests {
960903
id: DbId::from(0),
961904
name: "test_db".into(),
962905
tables: BTreeMap::new(),
906+
table_map: {
907+
let mut map = BiHashMap::new();
908+
map.insert(TableId::from(1), "test_table_1".into());
909+
map.insert(TableId::from(2), "test_table_2".into());
910+
map
911+
},
963912
};
964913
use InfluxColumnType::*;
965914
use InfluxFieldType::*;
@@ -1106,6 +1055,7 @@ mod tests {
11061055
id: DbId::from(0),
11071056
name: "test".into(),
11081057
tables: BTreeMap::new(),
1058+
table_map: BiHashMap::new(),
11091059
};
11101060
database.tables.insert(
11111061
TableId::from(0),
@@ -1142,6 +1092,11 @@ mod tests {
11421092
id: DbId::from(0),
11431093
name: "test_db".into(),
11441094
tables: BTreeMap::new(),
1095+
table_map: {
1096+
let mut map = BiHashMap::new();
1097+
map.insert(TableId::from(1), "test_table_1".into());
1098+
map
1099+
},
11451100
};
11461101
use InfluxColumnType::*;
11471102
use InfluxFieldType::*;
@@ -1188,6 +1143,11 @@ mod tests {
11881143
id: DbId::from(0),
11891144
name: "test_db".into(),
11901145
tables: BTreeMap::new(),
1146+
table_map: {
1147+
let mut map = BiHashMap::new();
1148+
map.insert(TableId::from(0), "test".into());
1149+
map
1150+
},
11911151
};
11921152
use InfluxColumnType::*;
11931153
use InfluxFieldType::*;

‎influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__catalog_serialization.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,5 @@ expression: catalog
156156
"sequence": 0,
157157
"host_id": "dummy-host-id",
158158
"instance_id": "instance-id",
159-
"db_map": [],
160-
"table_map": []
159+
"db_map": []
161160
}

‎influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_last_cache.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,5 @@ expression: catalog
8181
"sequence": 0,
8282
"host_id": "dummy-host-id",
8383
"instance_id": "instance-id",
84-
"db_map": [],
85-
"table_map": []
84+
"db_map": []
8685
}

‎influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_series_keys.snap

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,5 @@ expression: catalog
7070
"sequence": 0,
7171
"host_id": "dummy-host-id",
7272
"instance_id": "instance-id",
73-
"db_map": [],
74-
"table_map": []
73+
"db_map": []
7574
}

‎influxdb3_server/src/http.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,9 @@ where
700700
let table_id = self
701701
.write_buffer
702702
.catalog()
703-
.table_name_to_id(db_id, table.as_str().into())
703+
.db_schema(&db_id)
704+
.expect("db should exist")
705+
.table_name_to_id(table.as_str().into())
704706
.ok_or_else(|| WriteBufferError::TableDoesNotExist)?;
705707
match self
706708
.write_buffer
@@ -748,7 +750,9 @@ where
748750
let table_id = self
749751
.write_buffer
750752
.catalog()
751-
.table_name_to_id(db_id, table.into())
753+
.db_schema(&db_id)
754+
.expect("db should exist")
755+
.table_name_to_id(table.into())
752756
.ok_or_else(|| WriteBufferError::TableDoesNotExist)?;
753757
self.write_buffer
754758
.delete_last_cache(db_id, table_id, &name)

‎influxdb3_server/src/query_executor.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,7 @@ impl Database {
377377

378378
async fn query_table(&self, table_name: &str) -> Option<Arc<QueryTable>> {
379379
let table_name = table_name.into();
380-
let table_id = self
381-
.write_buffer
382-
.catalog()
383-
.table_name_to_id(self.db_schema.id, Arc::clone(&table_name))?;
380+
let table_id = self.db_schema.table_name_to_id(Arc::clone(&table_name))?;
384381
self.db_schema.get_table_schema(table_id).map(|schema| {
385382
Arc::new(QueryTable {
386383
db_schema: Arc::clone(&self.db_schema),
@@ -515,10 +512,7 @@ impl SchemaProvider for Database {
515512
}
516513

517514
fn table_exist(&self, name: &str) -> bool {
518-
self.write_buffer
519-
.catalog()
520-
.table_name_to_id(self.db_schema.id, name.into())
521-
.is_some()
515+
self.db_schema.table_name_to_id(name.into()).is_some()
522516
}
523517
}
524518

‎influxdb3_server/src/system_tables/parquet_files.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ impl IoxSystemTable for ParquetFilesTable {
9595
self.db_id,
9696
self.buffer
9797
.catalog()
98-
.table_name_to_id(self.db_id, table_name.as_str().into())
98+
.db_schema(&self.db_id)
99+
.expect("db exists")
100+
.table_name_to_id(table_name.as_str().into())
99101
.expect("table exists"),
100102
);
101103

0 commit comments

Comments
 (0)