Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass Session to all Encoders/Decoders allowing UDT Encoding without local session varaible in contexts e.g. ZIO and others #2219

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

deusaquilus
Copy link
Collaborator

@deusaquilus deusaquilus commented Aug 2, 2021

One significant limitation that Quill Encoders/Decoders have always had is that they have no access to the underlying session, only to the prepared statement. In many databases however, setting certain kinds of values on a prepared statement requires access to the session. For example, in Cassandra, creating a UDT requires access to the cluster:

val keyspace = cluster.getMetadata.getKeyspace("myKeyspace")
val udt: UdtValue = keyspace.getUserType("myUdt").newValue()

For this reason, the method udtValueOf was defined on CassandraSession which has access to the Cluster object.

One important problem with the Cassandra contexts and UDTs however is that they rely on the method udtValueOf to exist directly on the Cassandra contexts (see the UdtEncodingMacro which referenced q"$prefix.udtValueOf" in several places, where '$prefix' is the surrounding Cassandra Context). Most of the contexts that extend CassandraSession e.g. CassandraAsyncContext, CassandraSyncContext, CassandraMonixContext etc... however this is not true of the CassandraZioContext implementations which do not store the Cluster object locally. Rather, they return a ZIO object which has a CassandraZioSession dependency (this is defined as the CIO[T] type in CassandraZioContext). This means that for the ZIO contexts, the UDT functionality do not work (* unless a nasty workaround is employed, more on that later).

  • One possible workaround is to create a custom cassandra context (extending the CassandraZioContext) which actually has a udtValueOf method inside.

Also note that this problem has always existed in the CassandraLagom contexts. Those have never been able to use UDT functionality in the first place.

Now if the quill encoders could actually access the session, the UdtEncodingMacro could then use the session varaible for the encoders directly. This macro essentially syntheizes the encoders with the encode function therefore if the Session (i.e. the CassandraZioContext or others in our case) is available on that function, it could be used directly. Also note that some additional refactoring of CassandraMapper would be required for this to be possible for mapped for mapped UDTs as well.

Note however that if we add the Session to the Encoders, if it is not added to the Decoders as well, then certain problems arise. In Cassandra for example we need to modify the CassandraMapper to carry the Session value but if is is only available on the Encoding side, some kind of optional value (or the infamous null) has to be used in it's place. One other possibility is to have a variation of CassandraMapperWithUdt but then all the different cassandra collection encoders need to be implemented with both CassandraMapper and CassandraMapperWithUdt. Since the cassandra Map encoder takes a CassandraMapper for both keys and values, you need to implement all four variations. More pressingly, the UdtEncodingMacro itself would have to be implemented via both CassandraMapper and CassandraMapperWithUdt. For this reason, in order to maintain symmetry, the session must be passed into both the encders and decoders from whence it can be propogated into CassandraMapper and other places.

Note that this issue is hardly exclusive to Cassandra contexts or even ZIO. One problem that Quill has had is the inability to simply create a CLOB encoder because the CLOB must be created from the Session:

val conn: Connection = ...
val myClob: Clob = conn.createClob()
val ps: PreparedStatement = conn.prepareStatement(...)
val reader = new FileReader("file.txt")
ps.setClob(1, myClob)

If a clob encoder were to be written, we would need to be able to create it from the underlying connection. However, since currently, encoders do not have access to the underlying connection, this cannot be done in any straightfoward way.

Now the answer to all of these issues is simple, change the BaseEncoder and BasedDecoder:

// From:
type BaseEncoder[T] = (Index, T, PrepareRow) => PrepareRow
type BaseDecoder[T] = (Index, ResultRow) => T
// To:
type BaseEncoder[T] = (Index, T, PrepareRow, Session) => PrepareRow
type BaseDecoder[T] = (Index, ResultRow, Session) => T

This requires changes to every single context's implementation of encdoers and decoders as well as all of the tests.

Additionally in order for the Contexts to have the Session available in the execute___ methods, they need to have recieved it from the extractors and the prepares. That means that in Context.scala, they must be changed

// From this:
type Prepare = (PrepareRow) => (List[Any], PrepareRow)
type Extractor[T] = (ResultRow) => T
// To this:
type Prepare = (PrepareRow, Session) => (List[Any], PrepareRow)
type Extractor[T] = (ResultRow, Session) => T

Furthermore, the identity encoders all need to change as well:

// From this:
  protected val identityPrepare: Prepare = (p: PrepareRow) => (Nil, p)
  protected val identityExtractor = (rr: ResultRow) => rr
// To this:
  protected val identityPrepare: Prepare = (p: PrepareRow, s: Session) => (Nil, p)
  protected val identityExtractor = (rr: ResultRow, s: Session) => rr

The net of all of these changes is truly a significant refactoring touching the entirely of Quill. However because of the reasons mentioned above, I believe it is the right thing to do for the library as a whole.

  • Unit test all changes
  • Update README.md if applicable
  • Add [WIP] to the pull request title if it's work in progress
  • Squash commits that aren't meaningful changes
  • Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

@deusaquilus deusaquilus changed the title [WIP] Pass Session to all Encoders/Decoders allowing UDT Encoding to work properly with ZIO and More Contexts in Future [WIP] Pass Session to all Encoders/Decoders allowing UDT Encoding without local session varaible in contexts e.g. ZIO and others Aug 2, 2021
@deusaquilus deusaquilus force-pushed the prepare-with-session branch 2 times, most recently from e4a340f to 49bf6d9 Compare August 3, 2021 04:33
@deusaquilus deusaquilus changed the title [WIP] Pass Session to all Encoders/Decoders allowing UDT Encoding without local session varaible in contexts e.g. ZIO and others Pass Session to all Encoders/Decoders allowing UDT Encoding without local session varaible in contexts e.g. ZIO and others Aug 3, 2021
@deusaquilus deusaquilus merged commit 2174810 into master Aug 3, 2021
voropaevp pushed a commit to voropaevp/quill that referenced this pull request Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant