#2022 spark-commons #2023
#2022 spark-commons #2023
Conversation
AdrianOlosutean
added
the
work in progress
AdrianOlosutean
removed
the
work in progress
| .withColumnIfDoesNotExist(InfoDateColumn, to_date(lit(reportDate), ReportDateFormat)) | ||
| .withColumnIfDoesNotExist(InfoDateColumnString, lit(reportDate)) | ||
| .withColumnIfDoesNotExist(InfoVersionColumn, lit(reportVersion)) | ||
| .withColumnIfDoesNotExist(function)(InfoDateColumn, to_date(lit(reportDate), ReportDateFormat)) |
There was a problem hiding this comment.
It is late, but I think I take issue with DataFrameImplicits.DataFrameEnhancements.withColumnIfDoesNotExist(ifExists: (DataFrame, String) => DataFrame)(colName: String, colExpr: Column) that originated in AbsaOSS/spark-commons#18, apparently.
IMHO the name of the method simply does not conform to what it does and what params it requires - the name suggests no-op column does not exist, but it allows an action in such a case, too. My vote is either to have two separate methods withColumnIfDoesNotExist and transformIfColumnExists or at least rename this method somehow to make sense.
Here, you can alleviate the problem only by renaming the function to something more descriptive like ifExistsFn or similar. (this occurs in multiple codebase files)
There was a problem hiding this comment.
I tend to agree. In minimum, the parameters order seems wrong, and could have a default value in the ifExists parameter.
...jobs/src/main/scala/za/co/absa/enceladus/standardization/interpreter/stages/TypeParser.scala
Outdated
Show resolved
Hide resolved
…/interpreter/stages/TypeParser.scala Co-authored-by: Daniel K <dk1844@gmail.com>
spark-jobs/src/main/scala/za/co/absa/enceladus/conformance/HyperConformance.scala
Outdated
Show resolved
Hide resolved
benedeki
added
the
PR:reviewing
| import org.apache.spark.sql.types._ | ||
| import scala.util.Try | ||
|
|
||
| object StructFieldImplicits { |
There was a problem hiding this comment.
Why did you remove this functions? Within Enceladus they seemed/seems pretty useful - code is simpler and shorter.
There was a problem hiding this comment.
They are already in spark-commons, just a bit renamed and on Metadata. I'm not sure if it makes sense to be duplicated in Enceladus and spark-commons
There was a problem hiding this comment.
Hm, I see. They are useful, but I like them even better in the original form, being more concise. 🤷♂️
utils/src/main/scala/za/co/absa/enceladus/utils/schema/SparkUtils.scala
Outdated
Show resolved
Hide resolved
benedeki
removed
the
PR:reviewing
|
Kudos, SonarCloud Quality Gate passed!
|
benedeki
added
the
PR:no testing needed
|
Refactoring, doesn't really need/can be retested. |
Closes #2022