doobie icon indicating copy to clipboard operation
doobie copied to clipboard

Analysis produces false positives for enums in a non-default schema

Open nigredo-tori opened this issue 3 years ago • 5 comments

With Doobie 0.12.1 and PostgreSQL 11. Given this setup:

create schema custom;
create type custom.foo as enum('Bar');
create type foo as enum('Bar');

an analysis of a simple query using the type in the custom schema produces an error:

scala> val inCustom = sql"select 'Bar'::custom.foo".query[Foo].analysis.transact(xa).unsafeRunSync
inCustom: doobie.util.analysis.Analysis = Analysis(select 'Bar'::custom.foo,List(),List(Both((Basic(NonEmptyList(Some(Foo), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@613d7c91),NoNulls),ColumnMeta(Other,"custom"."foo",NullableUnknown,foo))))

scala> inCustom.columnTypeErrors
res35: List[doobie.util.analysis.ColumnTypeError] = List(ColumnTypeError(1,Basic(NonEmptyList(Some(Foo), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@613d7c91),NoNulls,ColumnMeta(Other,"custom"."foo",NullableUnknown,foo)))

A similar query using the type in the default schema (public) doesn't trigger this error:

scala> val inPublic = sql"select 'Bar'::foo".query[Foo].analysis.transact(xa).unsafeRunSync
inPublic: doobie.util.analysis.Analysis = Analysis(select 'Bar'::foo,List(),List(Both((Basic(NonEmptyList(Some(Foo), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@613d7c91),NoNulls),ColumnMeta(VarChar,foo,NullableUnknown,foo))))

scala> inPublic.columnTypeErrors
res36: List[doobie.util.analysis.ColumnTypeError] = List()

The difference seems to be caused by the JDBC type in the ColumnMeta. For inCustom it's Other, but for inPublic it's VarChar.

There is a possibility that this is caused by a driver bug rather then by a Doobie bug - but I don't have the time to dig deeper at the moment, and I need something to link to. :smile:

nigredo-tori avatar Jun 02 '21 10:06 nigredo-tori

As a hacky workaround, I've added a wrapper like this:

final case class AnalysisWorkaround[A](
  value: A,
  tweak: Analysis => Analysis
)

, which tweaks the produced Analysis in its Analyzable instance. The tweak itself is to replace JdbcType.Other with JdbcType.VarChar in ColumnMetas where vendorTypeName matches the expected string ("$schemaName"."$typeName").

nigredo-tori avatar Jun 03 '21 05:06 nigredo-tori

Yeah this seems like a JDBC driver bug. I've seen some issue in the past around pgjdbc and enums where enums from different schemas but with the same name are confused with one another. (Probably unrelated to your problem, but that area of the pgjdbc code is definitely...shady :grimacing: )

jatcwang avatar Jun 03 '21 09:06 jatcwang

This seems to be resolved by now. The issue, as reported by @nigredo-tori, is no longer appearing. Tested with Postgres 14.2, doobie-core 1.0.0-RC2, doobie-postgres 1.0.0-RC2.

DB setup:

# with postgres running on port 5432:
createuser --createdb enum_user
createdb -O enum_user enum_db

psql -U enum_user enum_db

# inside psql:
enum_db=> create schema custom;
create type custom.foo as enum('Bar');
create type foo as enum('Bar');

in sbt console

// ...
// [info] welcome to sbt 1.7.1 ...
// ...
// Welcome to Scala 2.13.8 (OpenJDK 64-Bit Server VM, Java 17.0.2).

scala> import cats._
     | import cats.data._
     | import cats.effect._
     | import cats.implicits._
     | import doobie._
     | import doobie.implicits._
     | import doobie.postgres._
     | import doobie.postgres.implicits._
     |
     | import doobie.util.ExecutionContexts
     |
     | import cats.effect.unsafe.implicits.global
     |
import cats._
import cats.data._
import cats.effect._
import cats.implicits._
import doobie._
import doobie.implicits._
import doobie.postgres._
import doobie.postgres.implicits._
import doobie.util.ExecutionContexts
import cats.effect.unsafe.implicits.global

scala> val xa = Transactor.fromDriverManager[IO](
     |   "org.postgresql.Driver",
     |   "jdbc:postgresql://localhost:5432/enum_db",
     |   "enum_user",
     |   ""
     | )
val xa: doobie.util.transactor.Transactor.Aux[cats.effect.IO,Unit] = doobie.util.transactor$Transactor$$anon$13@7c8ec2d7

scala> object Foo extends Enumeration {
     |   val Bar = Value
     | }
     |
object Foo

scala> implicit val FooMeta = pgEnum(Foo, "foo")
val FooMeta: doobie.Meta[Foo.Value] = doobie.util.meta.Meta@31e15092

// no errors using public schema:
scala> val inCustom1 = sql"select 'Bar'::foo".query[Foo.Value].analysis.transact(xa).unsafeRunSync()
val inCustom1: doobie.util.analysis.Analysis = Analysis(select 'Bar'::foo,List(),List(Both((Basic(NonEmptyList(Some(e.Value), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@295ee292),NoNulls),ColumnMeta(VarChar,foo,NullableUnknown,foo))))

scala> inCustom1.columnTypeErrors
val res1: List[doobie.util.analysis.ColumnTypeError] = List()

// no errors using custom schema
scala> val inCustom2 = sql"select 'Bar'::custom.foo".query[Foo.Value].analysis.transact(xa).unsafeRunSync()
val inCustom2: doobie.util.analysis.Analysis = Analysis(select 'Bar'::custom.foo,List(),List(Both((Basic(NonEmptyList(Some(e.Value), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@295ee292),NoNulls),ColumnMeta(VarChar,"custom"."foo",NullableUnknown,foo))))

scala> inCustom2.columnTypeErrors
val res2: List[doobie.util.analysis.ColumnTypeError] = List()

Comparing analysis results side-by-side:

// before:
Analysis(select 'Bar'::custom.foo,List(),List(Both((Basic(NonEmptyList(Some(Foo),     Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@613d7c91),NoNulls),ColumnMeta(Other,"custom"."foo",NullableUnknown,foo))))

// now:
Analysis(SELECT 'Bar'::custom.foo,List(),List(Both((Basic(NonEmptyList(Some(e.Value), Some(String)),NonEmptyList(Other, VarChar),List(),cats.free.Coyoneda$$anon$2@5a1c6d94),NoNulls),ColumnMeta(VarChar,"custom"."foo",NullableUnknown,foo))))

So now column meta is no longer Other but VarChar.

@jatcwang IMO this issue can be closed as per the above.

mkows avatar Nov 22 '22 10:11 mkows

Thanks you for the detailed report @mkows 🙏

jatcwang avatar Apr 11 '24 09:04 jatcwang

Reopening as I want to make sure there's a test for this

jatcwang avatar Apr 11 '24 09:04 jatcwang