Fix: Throw IllegalArgumentException for null IDs/Src/Dst in toGraphX (Fixes #765) by JeganVG · Pull Request #766 · graphframes/graphframes · GitHub
Skip to content

Fix: Throw IllegalArgumentException for null IDs/Src/Dst in toGraphX (Fixes #765)#766

Merged
SemyonSinchenko merged 6 commits intographframes:mainfrom
JeganVG:fix/765-null-id-handling
Jan 3, 2026
Merged

Fix: Throw IllegalArgumentException for null IDs/Src/Dst in toGraphX (Fixes #765)#766
SemyonSinchenko merged 6 commits intographframes:mainfrom
JeganVG:fix/765-null-id-handling

Conversation

@JeganVG
Copy link
Copy Markdown
Contributor

@JeganVG JeganVG commented Jan 2, 2026

…(for #765)

What changes were proposed in this pull request?

  1. Updated the pattern matching logic in toGraphX (for both vertices and edges) to explicitly catch null values.
  2. Replaced the generic GraphFramesUnreachableException with a meaningful IllegalArgumentException when null IDs are encountered.

Added Two TCs :

  1. toGraphX should throw IllegalArgumentException for null IDs
  2. toGraphX should throw IllegalArgumentException for null Edge Src/Dst

Why are the changes needed?

Fixes #765
Currently, if a GraphFrame contains null values in the ID (or src/dst) columns and toGraphX is called, the pattern matching falls, throwing a GraphFramesUnreachableException without proper exception.

@SemyonSinchenko
Copy link
Copy Markdown
Collaborator

@SemyonSinchenko
Copy link
Copy Markdown
Collaborator

We have both scalafmt and scalafix. If you do not want to use pre-commit, just call sbt scalafixAll

@JeganVG
Copy link
Copy Markdown
Contributor Author

JeganVG commented Jan 3, 2026

Hi @SemyonSinchenko,

My bad, I ran the comment and I didn't noticed the error after formatting was done, scalafix and scalafmt passed, I have rebuilt the PR, could you check ?

@JeganVG
Copy link
Copy Markdown
Contributor Author

JeganVG commented Jan 3, 2026

@SemyonSinchenko ,

updated the logic, verified test cases and precommit checks locally, could you check ? ;(

}

// 2. Null Src/Dst
val badEdges = edges.filter(col(SRC).isNull || col(DST).isNull).limit(1).count()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tbh, I do not like it. It is an expensive operation. I would prefer just to raise an exception instead. If no match: an exception that graph is malformed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I have rebuilt the PR, could you check ?

@codecov-commenter
Copy link
Copy Markdown

Copy link
Copy Markdown
Collaborator

@SemyonSinchenko SemyonSinchenko left a comment

Choose a reason for hiding this comment

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

@JeganVG Thanks for the contribution!

@SemyonSinchenko SemyonSinchenko merged commit 77c5599 into graphframes:main Jan 3, 2026
5 checks passed
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.

bug: it is possible to reach GraphFramesUnreachableException

3 participants