Skip to content

Conversation

@AdrianOlosutean
Copy link
Contributor

Closes #17

* @param schema An optional schema to validate if the column already exists (a very low probability)
* @return A name that can be used as a unique column name
*/
def getUniqueName(prefix: String, schema: Option[StructType]): String = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this an implicit on StructType. When not checked against a schema it's so trivial, I wouldn't event bother making it a common function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be removed since it will be replaced by this

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how the linked code solves for the need of unique column name? 🤔

Comment on lines 83 to 84
log.warn(s"Column '$colName' already exists. The content of the column will be overwritten.")
overwriteWithErrorColumn(df, colName, colExpr)
Copy link
Contributor

Choose a reason for hiding this comment

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

This again is pretty Encaladus specific. I would make this branch of code and input parameter.

ifExists: (DataFrame, String) => Unit = (_, _) => {}

This would also eliminate the need for the specific errorColumn code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So remove all the overwriteWithErrorColumn function?

@AdrianOlosutean AdrianOlosutean merged commit 7f58d55 into master Jan 25, 2022
@AdrianOlosutean AdrianOlosutean deleted the feature/17-dataframe-implicits branch February 17, 2022 05:52
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.

Add DataframeImplicits

3 participants