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

SPARKC-504: Save null UDTs #1228

Closed
wants to merge 11 commits into from
Closed

SPARKC-504: Save null UDTs #1228

wants to merge 11 commits into from

Conversation

acustiq
Copy link

@acustiq acustiq commented Feb 6, 2020

No description provided.

@datastax-bot
Copy link

Hi @acustiq, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign Spark Cassandra Connector CLA. It's all electronic and will take just minutes.

@datastax-bot
Copy link

Thank you @acustiq for signing the Spark Cassandra Connector CLA.

@RussellSpitzer
Copy link
Contributor

Sorry, I really messed up the master branch so a rebase will be necessary. If you could please rebase on b3.0 and I will review as soon as possible.

@@ -208,7 +208,7 @@ object MappedToGettableDataConverter extends StrictLogging {
for (i <- columnValues.indices)
columnValues(i) = converters(i).convert(columnValues(i))
struct.newInstance(columnValues: _*)
case None =>
case _ =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe i'm just paranoid, but should we do None | null? I may just be paranoid (this wouldn't be the first time...)

Copy link
Author

@acustiq acustiq Mar 30, 2020

Choose a reason for hiding this comment

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

I would think that the _ wildcard symbol catches both None and null as it matches anything:

case _ => // Wild card pattern -- matches anything

Source: https://docs.scala-lang.org/tutorials/FAQ/finding-symbols.html

@RussellSpitzer
Copy link
Contributor

All tests pass! Thanks so much for your pr, please let me know you response to my above comment. I'll change the target of this pr to b3.0 to make sure it gets in the next release.

@RussellSpitzer RussellSpitzer changed the base branch from master to b3.0 March 30, 2020 22:00
@RussellSpitzer
Copy link
Contributor

Ah we have a few of the master commits (just docs) got dragged in, could you please rebase on b3.0?

@acustiq
Copy link
Author

acustiq commented Mar 31, 2020

@RussellSpitzer please close this PR in favor of #1241 which has this change rebased on b3.0 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants