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

Version upgrade to Scala 2.12.17, Spark 3.4.1, Delta 2.4.0. #35

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ivan-veselovsky
Copy link

No description provided.

@ivan-veselovsky ivan-veselovsky force-pushed the version-upgrade-scala-2.12.15-spark-3.3.0 branch from 38f7733 to 6e7d54e Compare August 8, 2023 13:53
@ivan-veselovsky ivan-veselovsky changed the title Version upgrade scala 2.12.15 spark 3.3.0 Version upgrade to Scala 2.12.15, Spark 3.4.1, Delta 2.4.0. Aug 25, 2023
@ivan-veselovsky ivan-veselovsky changed the title Version upgrade to Scala 2.12.15, Spark 3.4.1, Delta 2.4.0. Version upgrade to Scala 2.12.17, Spark 3.4.1, Delta 2.4.0. Aug 31, 2023
Copy link
Contributor

@parisni parisni left a comment

Choose a reason for hiding this comment

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

many thanks for your work, one remark about commenting stuff, but overall LGTM

@@ -951,10 +951,10 @@ class DdlTest extends QueryTest with SparkSessionTestWrapper {
"FLOAT_COL",
"DOUBLE_COL",
"STRING_COL",
"DATE_COL",
"TIMESTAMP_COL",
//"DATE_COL",
Copy link
Contributor

Choose a reason for hiding this comment

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

do you have some context for commenting this ?

Copy link
Author

@ivan-veselovsky ivan-veselovsky Sep 8, 2023

Choose a reason for hiding this comment

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

Problem is that with updated Spark it becomes impossible to have in Dataset columns with types Array(Option(X)), where X is java.sql.Date, java.sql.Timestamp, scal.math.BigDecimal. E.g the following code fragment

Seq(Array(Some(new Date(1L)))).toDS()

causes error org.apache.spark.SparkRuntimeException: Error while encoding: java.lang.RuntimeException: scala.Some is not a valid external type for schema of date .

I commented the problematic lines, because, to be honest, I'm not sure how to fix this properly.
Simplest solution would be to remove these commented lines.
Any suggestions?

Copy link
Author

Choose a reason for hiding this comment

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

Removed commented code.

@ivan-veselovsky
Copy link
Author

many thanks for your work, one remark about commenting stuff, but overall LGTM

We use this project in our work, so thank you!

One more question: do you have any plans to release this library onto Maven Central ?

@parisni
Copy link
Contributor

parisni commented Sep 9, 2023 via email

@ivan-veselovsky
Copy link
Author

One more question: do you have any plans to release this library onto Maven Central ?
That would be great. Not sure every module has value. Which feature are you using so far ?

On September 8, 2023 12:10:55 PM UTC, Ivan @.> wrote: > many thanks for your work, one remark about commenting stuff, but overall LGTM We use this project in our work, so thank you! One more question: do you have any plans to release this library onto Maven Central ? -- Reply to this email directly or view it on GitHub: #35 (comment) You are receiving this because you commented. Message ID: @.>

So far we use spark-postgres (depends on spark-dataframe).

Copy link
Contributor

@parisni parisni left a comment

Choose a reason for hiding this comment

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

LGTM thanks a lot

@ivan-veselovsky
Copy link
Author

Super! Could we merge it?

@parisni
Copy link
Contributor

parisni commented Feb 9, 2024

sadly not. maybe @Dubrzr can ?

@ivan-veselovsky
Copy link
Author

Hello, guys, @parisni, @Dubrzr , can anybody please merge this PR?

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.

2 participants