Failed to find a type oid for ENUM_TYPE

Versions

Rust: 1.72
Diesel: 2.1
Diesel_async: 0.4.1
Database: postgresql
Operating System linux

Feature Flags

Problem Description

After I upgrade the code from diesel-async 0.3.x to 0.4, I faced this error: Failed to find a type oid for ENUM_TYPE.
After I downgrade, the error disappeared. The only change in the code base was diesel-async version.

Can you please provide an reproducible example, otherwise this report is not actionable.

I tested it on a big project.
I'll try to make a minimum code base next week 🙏

Thanks for working on a minimal example 👍

I had another look at the code base and there is a test for custom types here: https://github.com/weiznich/diesel_async/blob/ec38eca3d7fccf4097fae20d655a26f11df015ee/tests/custom_types.rs
That test passes with the last release, so it might be a problem specific to your type.

@weiznich, thanks. The problem is in eq_any (eq is fine).

If you add the following code to custom_types_round_trip test, it fails:

custom_types::table
        .filter(custom_types::custom_enum.eq_any([MyEnum::Foo]))
        .load(connection)
        .await
        .unwrap();

with this error:

thread 'custom_types::custom_types_round_trip' panicked at 'called `Result::unwrap()` on an `Err` value: 
SerializationError(FailedToLookupTypeError(PgMetadataCacheKey { schema: None, type_name: "my_type" }))'

Thanks for providing this example. I can reproduce the issue with that.

I confirm the bug is reproducible

The problematic code here is that one:

diesel_async/src/pg/mod.rs

Lines 319 to 382 in ec38eca

    
           let is_safe_to_cache_prepared = query.is_safe_to_cache_prepared(&diesel::pg::Pg); 
        
           let mut query_builder = PgQueryBuilder::default(); 
        
           let sql = query 
        
               .to_sql(&mut query_builder, &Pg) 
        
               .map(|_| query_builder.finish()); 
        
           let mut bind_collector = RawBytesBindCollector::<diesel::pg::Pg>::new(); 
        
           let query_id = T::query_id(); 
        
           // we don't resolve custom types here yet, we do that later 
        
           // in the async block below as we might need to perform lookup 
        
           // queries for that. 
        
           // 
        
           // We apply this workaround to prevent requiring all the diesel 
        
           // serialization code to beeing async 
        
           let mut metadata_lookup = PgAsyncMetadataLookup::new(); 
        
           let collect_bind_result = 
        
               query.collect_binds(&mut bind_collector, &mut metadata_lookup, &Pg); 
        
           let raw_connection = self.conn.clone(); 
        
           let stmt_cache = self.stmt_cache.clone(); 
        
           let metadata_cache = self.metadata_cache.clone(); 
        
           let tm = self.transaction_state.clone(); 
        
           async move { 
        
               let sql = sql?; 
        
               let is_safe_to_cache_prepared = is_safe_to_cache_prepared?; 
        
               collect_bind_result?; 
        
               // Check whether we need to resolve some types at all 
        
               // 
        
               // If the user doesn't use custom types there is no need 
        
               // to borther with that at all 
        
               if !metadata_lookup.unresolved_types.is_empty() { 
        
                   let metadata_cache = &mut *metadata_cache.lock().await; 
        
                   let mut next_unresolved = metadata_lookup.unresolved_types.into_iter(); 
        
                   for m in &mut bind_collector.metadata { 
        
                       // for each unresolved item 
        
                       // we check whether it's arleady in the cache 
        
                       // or perform a lookup and insert it into the cache 
        
                       if m.oid().is_err() { 
        
                           if let Some((ref schema, ref lookup_type_name)) = next_unresolved.next() { 
        
                               let cache_key = PgMetadataCacheKey::new( 
        
                                   schema.as_ref().map(Into::into), 
        
                                   lookup_type_name.into(), 
        
                               ); 
        
                               if let Some(entry) = metadata_cache.lookup_type(&cache_key) { 
        
                                   *m = entry; 
        
                               } else { 
        
                                   let type_metadata = lookup_type( 
        
                                       schema.clone(), 
        
                                       lookup_type_name.clone(), 
        
                                       &raw_connection, 
        
                                   ) 
        
                                   .await?; 
        
                                   *m = PgTypeMetadata::from_result(Ok(type_metadata)); 
        
                                   metadata_cache.store_type(cache_key, type_metadata); 
        
                               } 
        
                           } else { 
        
                               break; 
        
                           } 
        
                       } 
        
                   } 
        
               }

For custom types we need to resolve the type name to a oid, which might require executing a database query. I've choosen to delay resolving these oid's from the types names until we collected all bind parameters, but this has an important problem: Sometimes this oid is used during bind value serialization (for example for arrays: https://github.com/diesel-rs/diesel/blob/ac0b075f9ce297e06e8e728fce1573e7e1d8b797/diesel/src/pg/types/array.rs#L102) and then we don't get the serialized bind value at all, but an error message.

Now it's somewhat problematic to fix that because on the one hand we want to collect the bind parameters before entering the async below, to prevent a Send bound on the query generic type. On the other hand, we cannot execute queries outside an async block as executing a query requires resolving a future. I'm happy for suggestions on how to resolve that situation.

Attempted to port a decent-sized codebase over to diesel_async, and everything seems to have worked great except for this (which is pretty blocking). I'm not sure if I'll have time to try to come up with a solution or not yet, but wanted to at least report hitting it

It's on my list to address this at some point, but I cannot really make promises about the timeline. The problem is not that simple to solve is outlined in this comment. Essentially it boils down to the fact that we don't want/cannot have a Send bound on the query for reasons but that makes it harder to evaluate futures borrowing that type. In the end it's likely solvable by making the returned future conditional send based on the type parameters.

As a temporary workaround it should be possible to just have a query at connection setup that does use that enum type and doesn't use .eq_any(). That will prepopulate the cache and therefore workaround the cache miss in later queries. That's not a real solution but it should unblock any user that is affected by this bug.

As a temporary workaround it should be possible to just have a query at connection setup that does use that enum type and doesn't use .eq_any(). That will prepopulate the cache and therefore workaround the cache miss in later queries. That's not a real solution but it should unblock any user that is affected by this bug.

Has anyone gotten this to work? If so can anyone provide an example of what this looks like, I tried doing the following without much luck.

sql_query("SELECT COUNT(*) from segment_features WHERE feature_type = $1")
 .bind::<sql_types::SegmentFeatureType, _>(feature)
 .await
 .context(format!("Failed to prefetch {feature}"))?;

Has anyone gotten this to work? If so can anyone provide an example of what this looks like, I tried doing the following without much luck.
sql_query("SELECT COUNT(*) from segment_features WHERE feature_type = $1")
 .bind::<sql_types::SegmentFeatureType, _>(feature)
 .await
 .context(format!("Failed to prefetch {feature}"))?;

I haven't tried that. Because I have many enum types and the query is generated dynamically, so the workaround doesn't make sense that much in my case.

But anyway, you may need to get_result from the query you wrote. Or can you try a normal query with .eq()? I mean without sql_query.

And generally COUNT(*) is slow-ish, better to return a row and .limit(1). Something like:

_ = segment_features::table
    .select(segment_features::id)
    .filter(segment_features::feature_type.eq(feature))
    .limit(1)
    .get_result::<Uuid>(conn)
    .await;

@techtunde You are right, that also does not work. :(

@weiznich I do not have enough knowledge about the codebase, but I'm seriously considering moving to diesel-async on my company's project (because we're hitting a limit on spawn-blocking approach for regular diesel).

We rely heavily on enums, so this issue is likely a blocker for us! I wanted to try and help tackling it, but I don't know where to start. Do you have an idea of implementation or even an indication of where I should start reading to understand the issue?

@lsunsi please ask your company to sponsor @weiznich to push this forward 🙏🏼 and/or support him yourself

@omid This would be awesome and I'll pass this message along. In the in between, I might be able to help with some code myself, if that's useful for him or you. (:

@lsunsi See this post for a starting point: #103 (comment). I do not have a specific idea how to solve this yet.

Instead of resolving OIDs asynchronously, how about resolving it synchronously? This would only affect the performance of the initial query; every other query would have the same performance.

That's unfortunately no acceptable solution to the problem. If we have an async connection implementation it needs to be fully async, otherwise it's just not worth the trouble to maintain it at all.

That's unfortunately no acceptable solution to the problem. If we have an async connection implementation it needs to be fully async, otherwise it's just not worth the trouble to maintain it at all.

The connection doesn't need to be blocking. We just need to conditionally do a spawn_blocking with the underlying engine.

That's unfortunately not possible as the spawn_blocking requires a Send bound on all arguments and some of them are not Send.

Moreover I share the perception that it'd be surprising to see an spawn_blocking (wherever it is) in the async implementation, it'd have to always be cited as an "exception" to the async model.

I'm also hitting this issue. Is there a way to specify the oid manually ? We can always get it dynamically beforehand with SELECT 'my_type'::regclass::oid;

I'm using a custom composite type and not an enum but I don't think this makes a difference.

#[derive(SqlType)]
#[derive(QueryId)]
#[diesel(postgres_type(name = "my_type"))]
pub struct MyType;

As a temporary workaround it should be possible to just have a query at connection setup that does use that enum type and doesn't use .eq_any(). That will prepopulate the cache and therefore workaround the cache miss in later queries. That's not a real solution but it should unblock any user that is affected by this bug.

@weiznich Wouldn't it be possible to look up all oid's of enums at startup and put them in the cache so that they don't have to be resolved when encountered for the first time?
Something like: select oid, typname from pg_type where pg_type.oid = any (select enumtypid from pg_enum); ?

@DontBreakAlex There is no known workaround yet as already written above. Asking that again does likely not change anything, at least other than binding time that could be used to work on a solution.

@LorenzSchueler I remember that I tried that at some point, but it did not work in certain cases. If you create the enum after you established the connection for example, which is quite common if you run migrations from your application. The other disadvantage is this is a quite large amount of data. There are 699 entries in my local pg_type table. We would need to load them all, as we need to support other types than enums as well.

@weiznich I'm sorry if I hurt your feelings, I didn't mean to. I was thinking about doing this:

let type_id: TypeId = diesel::sql_query(
	r#"SELECT "pg_type"."oid" AS oid, "pg_type"."typarray" AS array_oid FROM "pg_type" WHERE typname = 'additional_field';"#,
)
.load(&mut conn).await.unwrap().pop().unwrap();
ADDITIONAL_TYPE_OID.store(type_id.oid, Ordering::Release);
ADDITIONAL_TYPE_ARRAY_OID.store(type_id.array_oid, Ordering::Release);
...
#[derive(QueryId)]
pub struct AdditionalFieldType;

impl diesel::sql_types::SqlType for AdditionalFieldType {
	type IsNull = diesel::sql_types::is_nullable::NotNull;
}
impl diesel::sql_types::SingleValue for AdditionalFieldType {}

impl diesel::sql_types::HasSqlType<AdditionalFieldType> for diesel::pg::Pg {
	fn metadata(_: &mut Self::MetadataLookup) -> PgTypeMetadata {
		PgTypeMetadata::new(ADDITIONAL_TYPE_OID.load(Ordering::Acquire), ADDITIONAL_TYPE_ARRAY_OID.load(Ordering::Acquire))
	}
}

This works nicely and I can refresh the oid whenever I need to

@weiznich I'm not so familiar with the code base of diesel and also diesel_async, and also not familiar with the low-level postgres queries. I spent some time today getting more familiar.

Now I have a question, why do we use binary format for arrays in postgres? https://github.com/diesel-rs/diesel/blob/ac0b075f9ce297e06e8e728fce1573e7e1d8b797/diesel/src/pg/types/array.rs#L102

Cannot we just build the simple text format? (like: select * from custom_types where custom_enum = any (ARRAY['foo']::my_type[]))

We generally use the binary protocol for all bind values because that decouples the values from the query, which in turn allows us not to care about possible injections. It also gives us better performance as the binary representation is usually smaller.

@weiznich thanks. Another question, somewhere in the code comments, you mentioned: We apply this workaround to prevent requiring all the diesel serialization code to be async.
What's the problem, to run it in async?
What will be the problem to not prevent a Send bound on the query generic type?

Another question, somewhere in the code comments, you mentioned: We apply this workaround to prevent requiring all the diesel serialization code to be async.
What's the problem, to run it in async?

The problem would be that the Pg::metadata function would need to return a future if we would execute an async query inside that function. That in turn would need ToSql::to_sql would need to return a future as well, which in turn would be either:

A breaking change to diesel itself (and problematic there, because what would we do with futures there?)
Disallow reusing the serialization code from diesel, which in turn would make diesel_async incompatible with the existing diesel ecosystem.

What will be the problem to not prevent a Send bound on the query generic type?

That likely would break the AsyncConnectionWrapper , as we don't have a +Send bound on the LoadConnection::load impl in diesel itself, as opposed to what we had on the AsyncConnection::load impl in diesel_async 0.3. Otherwise it would be "simple" to just perform the lookup queries while serializing the results, but that's not possible, because as soon as we .await and still hold any reference to the query we would need that Send bound there.

No way... Thanks @weiznich and @dullbananas. You are great <3

	let is_safe_to_cache_prepared = query.is_safe_to_cache_prepared(&diesel::pg::Pg);
	let mut query_builder = PgQueryBuilder::default();
	let sql = query
	.to_sql(&mut query_builder, &Pg)
	.map(\|_\| query_builder.finish());

	let mut bind_collector = RawBytesBindCollector::<diesel::pg::Pg>::new();
	let query_id = T::query_id();

	// we don't resolve custom types here yet, we do that later
	// in the async block below as we might need to perform lookup
	// queries for that.
	//
	// We apply this workaround to prevent requiring all the diesel
	// serialization code to beeing async
	let mut metadata_lookup = PgAsyncMetadataLookup::new();
	let collect_bind_result =
	query.collect_binds(&mut bind_collector, &mut metadata_lookup, &Pg);

	let raw_connection = self.conn.clone();
	let stmt_cache = self.stmt_cache.clone();
	let metadata_cache = self.metadata_cache.clone();
	let tm = self.transaction_state.clone();

	async move {
	let sql = sql?;
	let is_safe_to_cache_prepared = is_safe_to_cache_prepared?;
	collect_bind_result?;
	// Check whether we need to resolve some types at all
	//
	// If the user doesn't use custom types there is no need
	// to borther with that at all
	if !metadata_lookup.unresolved_types.is_empty() {
	let metadata_cache = &mut *metadata_cache.lock().await;
	let mut next_unresolved = metadata_lookup.unresolved_types.into_iter();
	for m in &mut bind_collector.metadata {
	// for each unresolved item
	// we check whether it's arleady in the cache
	// or perform a lookup and insert it into the cache
	if m.oid().is_err() {
	if let Some((ref schema, ref lookup_type_name)) = next_unresolved.next() {
	let cache_key = PgMetadataCacheKey::new(
	schema.as_ref().map(Into::into),
	lookup_type_name.into(),
	);
	if let Some(entry) = metadata_cache.lookup_type(&cache_key) {
	*m = entry;
	} else {
	let type_metadata = lookup_type(
	schema.clone(),
	lookup_type_name.clone(),
	&raw_connection,
	)
	.await?;
	*m = PgTypeMetadata::from_result(Ok(type_metadata));

	metadata_cache.store_type(cache_key, type_metadata);
	}
	} else {
	break;
	}
	}
	}
	}