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

spark agent #272 fix duplicated attribute ids #281

Merged
merged 2 commits into from
Aug 12, 2021

Conversation

cerveada
Copy link
Contributor

@cerveada cerveada commented Aug 12, 2021

  • remove deprecated symbol /:

- remove deprecated symbol :/
@cerveada cerveada marked this pull request as ready for review August 12, 2021 08:44
@cerveada cerveada requested a review from wajda August 12, 2021 08:44

val duplicates = operation.output.groupBy(_.exprId).collect { case (x, List(_,_,_*)) => x }
if (duplicates.nonEmpty) {
logError(s"Duplicated attributes found in Join operation output, ExprIds of duplicates: $duplicates")
Copy link
Contributor

Choose a reason for hiding this comment

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

We log it as an Error, but there is no recovery logic and we just carry on. Shouldn't we log it as Warning instead?


override def build(): DataOperation = {

val duplicates = operation.output.groupBy(_.exprId).collect { case (x, List(_,_,_*)) => x }
Copy link
Contributor

Choose a reason for hiding this comment

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

The operation.output type is Seq, but the List(_,_,_*) would only work for Lists, not for other Seqs.
We should use a more generic type destructor Seq(_, _, _*). Or even better, since we are only interested in count but not the values itself, we could communicate this logic more clearly:

Suggested change
val duplicates = operation.output.groupBy(_.exprId).collect { case (x, List(_,_,_*)) => x }
val duplicates = operation.output.groupBy(_.exprId).collect { case (exprId, attrs) if attrs.length > 1 => exprId }

@sonarcloud
Copy link

sonarcloud bot commented Aug 12, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@wajda wajda left a comment

Choose a reason for hiding this comment

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

LGTM

@cerveada cerveada merged commit d9887e3 into release/0.6 Aug 12, 2021
@cerveada cerveada deleted the bugfix/agent-272-duplicated-attributes branch August 12, 2021 10:54
@kuhnen
Copy link

kuhnen commented Aug 16, 2021

Amazing @cerveada :). Do you guys have plans to deploy a new version of the agent?

@wajda
Copy link
Contributor

wajda commented Aug 16, 2021

I'm working on it at the moment. Will trigger the release shortly after finishing some testing.

@kuhnen
Copy link

kuhnen commented Aug 16, 2021

amazing :). thanks a lot @wajda

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.

3 participants