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

Support user defined data types when describing table schema. #771

Merged
merged 17 commits into from
Nov 29, 2022

Conversation

sviezypan
Copy link
Collaborator

@sviezypan sviezypan commented Nov 18, 2022

This PR introduces new way to describe tables with the defineTable[A] method.

Old way to define table schema was by using DSL and ++ operator.

val customers =
      (uuid("id") ++ localDate(“date_of_birth”) ++ string("first_name") ++ string("last_name") ++ boolean("verified_customer") ++ zonedDateTime("created")).table("customers")

    val (customerId, dob, fName, lName, verified, createdString, createdTimestamp) = customers.columns

While composable, it was a bit boilerplate-y and another DSL to learn. I think that for Scala devs its more natural to write a case class, which could be then reused when doing select or insert with the same table.

  case class Customer(id: UUID, dob: LocalDate, firstName: String, lastName: String, verified: Boolean, 
createdTimestampString: String, createdTimestamp: ZonedDateTime)

We can get table description just by calling defineTable[Customer] method. The case class data type also needs an implicit schema (which is required for inserts anyway)

    implicit val custommerSchema = DeriveSchema.gen[Customer]

    val customers = defineTable[Customer]

    val (customerId, dob, fName, lName, verified, createdString, createdTimestamp) = customers.columns

There are 2 overrides
defineTableSmart[Customer] and defineTable[Customer]("customers"), where the first one smartly tries to make plural out of case class name, while taking into account also irregular english plural words.
OrderOrigin -> order_origins
PersonAddress -> person_addresses
Foot -> feet
defineTable[Customer]("customers") takes table name explicitly.
Field names are also converted from camelCase to snake_case
firstName -> first_name

There is also a macro which verifies that data type can be used to describe table ( sql does not know sum types). For example:

    sealed trait PaymentMethod
    case object Card extends PaymentMethod
    case object Cash extends PaymentMethod

    case class Bill(amount: Int, paymentMethod: PaymentMethod)
    
    val billTable = defineTable[Bill]

defineTable[Bill] will emit an compilation error
Unsupported types by SQL OrderDetails.PaymentMethod

@jdegoes @jczuchnowski let me know what you think. Docs are updated as well.

@sviezypan sviezypan requested a review from a team as a code owner November 18, 2022 14:17
@@ -4,6 +4,7 @@ import com.github.ghik.silencer.silent

import java.time._
import scala.language.implicitConversions
import java.math.BigDecimal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't seem to be needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we don't support scala.math.BigDecimal until I add TypeId to Schema.Transform. scala.math.BigDecimal is encoded as Schema.transform of java.math.BigDecimal and without TypeId on Transform I am not able to derive TypeTag for scala.math.BigDecimal.

Comment on lines -16 to -23
val productTable = (
string("id") ++
localDate("last_updated") ++
string("name") ++
int("base_amount") ++
int("final_amount") ++
boolean("deleted")
).table("product")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still have tests that are using the "old way".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not supporting ++ for definition of "source" tables. Let's discuss if there's a use case for having both.

builder.append(
s"""TO_TIMESTAMP_TZ('${formatter.format(
s"""TO_TIMESTAMP_TZ('${ISO_INSTANT.format(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +108 to +109
// TODO uncomment and fix java.sql.SQLDataException: ORA-01874: time zone hour must be between -15 and 15
// oracle won't run on m1 machine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a problem only on m1 machine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the test is failing everywhere but I am not able to reproduce on m1 because of oracle image not running at all.

Comment on lines -17 to -19
val customers =
(uuid("id") ++ localDate(“date_of_birth”) ++ string("first_name") ++ string("last_name") ++ boolean("verified_customer") ++ zonedDateTime("created"))
.table("customers")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later in section Subqueries & Correlated subqueries of this document, there are still examples of this kind of definitions but without explanation.
I think we should have both ways well documented. The ++ operator is still useful for defining simple ad-hoc queries for which we don't need to define a model.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I replaced ++ with data type definition, I now changed it in all places in docs.

@sviezypan sviezypan merged commit 2046195 into zio:master Nov 29, 2022
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.

None yet

2 participants