Check the table route type before calling physical_table_route

[WenyXu]

What type of enhancement is this?

Tech debt reduction

What does the enhancement do?

greptimedb/src/common/meta/src/key/table_route.rs

Lines 65 to 103 in feec4e2

	pub fn update(&self, region_routes: Vec<RegionRoute>) -> Self {
	let version = self.physical_table_route().version;
	Self::Physical(PhysicalTableRouteValue {
	region_routes,
	version: version + 1,
	})
	}
	/// Returns the version.
	///
	/// For test purpose.
	#[cfg(any(test, feature = "testing"))]
	pub fn version(&self) -> u64 {
	self.physical_table_route().version
	}
	/// Returns the corresponding [RegionRoute].
	pub fn region_route(&self, region_id: RegionId) -> Option<RegionRoute> {
	self.physical_table_route()
	.region_routes
	.iter()
	.find(|route| route.region.id == region_id)
	.cloned()
	}
	/// Gets the [RegionRoute]s of this [TableRouteValue::Physical].
	///
	/// # Panics
	/// The route type is not the [TableRouteValue::Physical].
	pub fn region_routes(&self) -> &Vec<RegionRoute> {
	&self.physical_table_route().region_routes
	}
	fn physical_table_route(&self) -> &PhysicalTableRouteValue {
	match self {
	TableRouteValue::Physical(x) => x,
	_ => unreachable!("Mistakenly been treated as a Physical TableRoute: {self:?}"),
	}
	}

We should add panic notes before methods that the inside invoke the physical_table_route. And add a check before we invoke these methods(e.g., matches!(table, TableRouteValue::pshyical(_))).

Implementation challenges

No response

[AntiTopQuark]

Hello, I would like to try this task, but I am not very familiar with Rust and Greptime, so it might take me a bit more time. Would that be acceptable to you?

[waynexia]

Great, feel free to reach out if you have anything want to discuss

[AntiTopQuark]

I made an attempt, could you please check it for me?
I have a question: since the physical_table_route() function already includes the unreachable! macro, why is there a need to add a check in the functions that call it at a higher level? Is it to detect errors earlier?

#3023

[WenyXu]

why is there a need to add a check in the functions that call it at a higher level?

It should throw an error instead of panic.