CodeLionX / actordb

Actor Database System Framework using Akka
MIT License
0 stars 2 forks source link

Relations allow inserting Records that follow a different schema than their own #10

Closed srfc closed 6 years ago

srfc commented 6 years ago

Issue

If a Record that is built using a different schema (i.e. ColumnDef sequence) than some Relation, it can still be inserted into said Relation.

Problem Description

If a Record that is built using a different schema (i.e. ColumnDef sequence) than some Relation, it can still be inserted into said Relation. All fields of the record, which do not correspond to a ColumnDef of the Relation are subsequently lost.

I propose adding a check on Relation.insert and Relation.insertAll if the Record.columns equal the Relation.columns and returning a Either[Error/InsertedRecord(s)]. Would this be the proper way to add success/failure feedback?

Supporting Information

val R: RowRelation = RowRelation(Seq(ColumnDef[Int]("id"), ColumnDef[Int]("age")))

R.insert(
  Record(Seq(ColumnDef[Int]("zipcode"), ColumnDef[String]("lastname")))
    .withCellContent(ColumnDef[Int]("zipcode") -> 13357)
    .withCellContent(ColumnDef[String]("lastname") -> "Meyer")
    .build()
)
// DOES NOT FAIL
// results in a row with all Some(null) values in R
CodeLionX commented 6 years ago

I like your idea using Either[T] as return type. I just want to throw in the type Try of the scala standard library:

The Try type represents a computation that may either result in an exception, or return a successfully computed value. It's similar to, but semantically different from the scala.util.Either type.

scala-lang API

CodeLionX commented 6 years ago

I've an additional question: What happens when you pass a Record with all columns of the Relation plus some more?

val R: RowRelation = RowRelation(Seq(ColumnDef[Int]("id"), ColumnDef[Int]("age")))

val result = R.insert(
  Record(Seq(ColumnDef[Int]("id"), ColumnDef[Int]("age"), ColumnDef[Int]("zipcode"), ColumnDef[String]("lastname")))
    .withCellContent(ColumnDef[Int]("id") -> 1)
    .withCellContent(ColumnDef[Int]("age") -> 23)
    .withCellContent(ColumnDef[Int]("zipcode") -> 13357)
    .withCellContent(ColumnDef[String]("lastname") -> "Meyer")
    .build()
)
println(result)
// Was the record successfully inserted, because it contained all columns of the relation?
srfc commented 6 years ago

I think this should fail. Ideally a user should always - in my mind at least - use the Relation he wants to insert data into to create new Record's. I.e.

val result = R.insert(
  R.newRecord  // this is what I mean
    .withCellContent(ColumnDef[Int]("id") -> 12412)
    .withCellContent(ColumnDef[String]("name") -> "Max Mustermann")
    .build()
)

At least I would encourage this, or the Record(R.columns) pattern. On each insert I would check if the column layout of the Record fits the Relation's.

Otherwise we will silently drop data that someone thinks they're pushing into the relation because they e.g. forgot to update the Relation's column layout and that would be bad.

CodeLionX commented 6 years ago

Alright, I agree.

+1 for:

val r = R.newRecord.build()
CodeLionX commented 6 years ago

we are now able to do a record.project(R):

val R: RowRelation = RowRelation(Seq(ColumnDef[Int]("id"), ColumnDef[Int]("age")))
val record = Record(Seq(
    ColumnDef[Int]("id"), 
    ColumnDef[Int]("age"), 
    ColumnDef[Int]("zipcode"), 
    ColumnDef[String]("lastname")
  ))(
    ColumnDef[Int]("id") ~> 1 &
    ColumnDef[Int]("age") ~> 23 &
    ColumnDef[Int]("zipcode") ~> 13357 &
    ColumnDef[String]("lastname") ~> "Meyer"
  ).build()

val result1 = R.insert(record)
// result1 == Failure
val result2 = R.insert(record.project(R))
// result2 == Success

This supports your point that all inserts with incompatible column definition sets should fail, as one can easily project the record to the relations definition (if the record contains all columns of the relation).