Skip to content

Conversation

@taherkl
Copy link

@taherkl taherkl commented Jan 8, 2025

This is an old bug which could only surface with the more recent addition of custom Cypher queries.

The template tries to pre-fetch source data.

It does it for text sources, since they do not support SQL pushdown, so their data is required for post-processing anyway.

Before this commit, the template also pre-fetched data when none of the source's targets defined source transformations.

This is overly restrictive and actually wrong.

Custom query targets cannot define source transformations.

If they share a source with a node/rel targets that define custom source transformations, then the template would crash with a NPE.

This is now fixed. Source data is pre-fetched as long as there is at least one of its target that does not define any transformation.

The commit also adds another small optimization: if the source does not match any active targets, the source processing is skipped completely. Before that, the data could be pre-fetched, incurring unnecessary data movement.

This is an old bug which could only surface with the more recent
addition of custom Cypher queries.

The template tries to pre-fetch source data.

It does it for text sources, since they do not support SQL
pushdown, so their data is required for post-processing anyway.

Before this commit, the template also pre-fetched data when none
of the source's targets defined source transformations.

This is overly restrictive and actually wrong.

Custom query targets cannot define source transformations.

If they share a source with a node/rel targets that define custom
source transformations, then the template would crash with a NPE.

This is now fixed. Source data is pre-fetched as long as there is
at least one of its target that does not define any transformation.

The commit also adds another small optimization: if the source
does not match any active targets, the source processing is
skipped completely. Before that, the data could be pre-fetched,
incurring unnecessary data movement.
Copy link
Collaborator

@pawankashyapollion pawankashyapollion left a comment

Choose a reason for hiding this comment

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

LGTM

@taherkl taherkl merged commit c6442ba into ollionorg:main Jan 8, 2025
3 of 4 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.

3 participants