frees-io / freestyle-cassandra

Freestyle Cassandra
http://frees.io/
Apache License 2.0
17 stars 4 forks source link

Add the Statement type to the interpolator response #98

Closed FPerezP closed 7 years ago

FPerezP commented 7 years ago

We are currently supporting SELECT, INSERT, DELETE and UPDATE operations. We want to have methods available to return a case class or a list of case classes - instead of ResultSet (ticket https://github.com/frees-io/freestyle-cassandra/issues/93) -, but for operations returning void, we should:

We first need to add the operation type (DataDefinition or DataManipulation) to the return type of the evaluate method: https://github.com/frees-io/freestyle-cassandra/blob/5e1f0d99870aa77c97c90c807ef5c3d3a6225bfc/core/src/main/scala/query/interpolator/CQLInterpolator.scala#L61-L67

Additionally, and since we want a common CQL string interpolator for both sort of operations, we'd like to have a common returning type for them. The common type is troy.cql.ast.Cql3Statement

So, the response type would it be:

-- def evaluate(interpolation: RuntimeInterpolation): (String, List[SerializableValueBy[Int]])
++ def evaluate(interpolation: RuntimeInterpolation): (String, Cql3Statement, List[SerializableValueBy[Int]])

You can use the methods CqlParser.parseSchema(cql) and CqlParser.parseDML(cql) for obtaining the DataDefinition and DataManipulation, potentially reusing the function defined here but chaining both calls (CqlParser.parseSchema(cql) and CqlParser.parseDML(cql)): https://github.com/frees-io/freestyle-cassandra/blob/5e1f0d99870aa77c97c90c807ef5c3d3a6225bfc/core/src/main/scala/query/interpolator/CQLInterpolator.scala#L45-L50

Then, the implicit class InterpolatorOps should be updated to match the new type (but we're not going to use the new data, this will be done on #107): https://github.com/frees-io/freestyle-cassandra/blob/4bd76315aab0684417036a5911577cc3be0cd537/core/src/main/scala/query/interpolator/package.scala#L33

The RuntimeCQLInterpolatorSpec test should be updated: https://github.com/frees-io/freestyle-cassandra/blob/9e217341a940c44d4fef91d9164157277c5d7ba6/core/src/test/scala/query/interpolator/RuntimeCQLInterpolatorSpec.scala#L28-L68 And potentially, InterpolatorImplicitSpec and CQLInterpolatorSpec would require some minor changes.

fagossa commented 7 years ago

Hello @FPerezP ,

First of all, thanks a lot for such a detailed report 👍

This is amy first time working on this repo, so I have several beginner questions regarding the proposed evaluate function:

def evaluate(interpolation: RuntimeInterpolation): (
      String,
      Try[Cql3Statement],
      List[SerializableValueBy[Int]])

If is safe to assume it, do we need to select a statement as default? I mean pick one from the ones available in troy?

val cql = interpolation.parts.foldLeft("") {
      case (prev, _ @Literal(_, string)) => prev + string
      case (prev, _ @Substitution(_, _)) => prev + "?"
      case (prev, _)                     => prev
}
def statement(cql: String) = CqlParser.parseDML(cql) match {
  // TODO: handle different cases
}

interpolation.parts.foldLeft(("", statement(cql), List.empty[SerializableValueBy[Int]])) {
...
}

Thanks a lot!

fedefernandez commented 7 years ago

Hi @fagossa, thanks for your interest in the project. I completed the ticket description so I'm the responsible for the possible incongruences :smile:

I assigned the ticket to myself because we figured out that it's not possible to solve the second statement (Disable as[Model] and asList[Model] methods) with implicit methods, because we'll be returning a Cql3Statement in the evaluate method and not a DataDefinition or a DataManipulation.

Answering your second question, and I think that also to the first one, we want to give support to data definition statements in the interpolator, this is why we need to try both validations, parseDML and parseSchema.

I've got a WIP here, feel free to take a look and add comments. It still pending the asFree implementation and add the test cases for data definition statements.

Please, if you want to collaborate, take a look at the help wanted tickets. The issues #95 #102 #104 and #105 are a nice starting point.

fedefernandez commented 7 years ago

Closing as we don't need this for any pending functionality. We would re-open this issue in the case is needed in a near future.