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

[FEEDBACK WANTED] Don't create column from dataset #216

Closed
wants to merge 1 commit into from

Conversation

frosforever
Copy link
Contributor

I may be missing some context here, but it seems like the current way of selecting a column can lead to runtime errors that the compiler okays.
Currently, columns are selected using the dataset's col method. The compile time check however only ensures that the field exists in the Dataset's type T. However, this does not prevent one from selecting a column from a different dataset of the same T at compile time resulting in a runtime blow up.

One way to get around this is started in this PR. Instead of selecting the dataset's col, create a general column instead. This would at least line up with what the compiler is proving, simply that a col of field foo exists in T and has the type U but not that the column requested is that specific dataset's column.

example:

val ds1: TypedDataset[X2[Int, Int]] = TypedDataset.create(Seq.empty[X2[Int, Int]])
val ds2: TypedDataset[X2[Int, Int]] = TypedDataset.create(Seq.empty[X2[Int, Int]])

//this will compile but blow up at runtime
ds2.filter(ds1('a) === 1)

Are there other ways to get around this?
Is this not considered an issue to be concerned about?

@frosforever
Copy link
Contributor Author

Never mind! Seems like #186, #187, #110 hit on this exact issue. My mistake for not seeing that earlier.

@frosforever frosforever closed this Dec 3, 2017
@frosforever frosforever deleted the column-selecting branch December 3, 2017 00:19
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