Skip to content

Conversation

@chrisvittal
Copy link
Collaborator

@chrisvittal chrisvittal commented Aug 2, 2024

As mentioned in #14580, IR can get quite big, especially as it can contain an arbitrary amount of encoded literals from the user's python session.

Tested manually, by making a very very large literal, running a pipeline with it on 0.2.132, observing the failure seen in #14650, then running the same pipeline with this change, and it succeeds as normal.

Resolves #14650

Copy link
Member

@ehigham ehigham left a comment

Choose a reason for hiding this comment

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

Can you comment about how you've tested this? I think it might also be worth commenting in the source code why/in what circumstances strings might get so long?

As an aside, do you know why strings are getting so crazy long? Is that legit?

@chrisvittal chrisvittal force-pushed the query/fix-other-stringlength-constraints branch from 9e89f35 to fd10c0b Compare August 5, 2024 16:18
@chrisvittal chrisvittal requested a review from ehigham August 5, 2024 16:19
@chrisvittal
Copy link
Collaborator Author

@ehigham Think I've addressed everything.

Copy link
Member

@ehigham ehigham left a comment

Choose a reason for hiding this comment

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

Thanks for adding the comments - that'll be helpful when reviewing this code in the future no doubt!

Copy link
Member

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

I'm a bit concerned about the replication, and in the future making a change and forgetting to apply it in all three places. Instead of doing this in the apply methods for each backend (and main for the service backend), could we move it to the constructor of the Backend superclass?

Copy link
Member

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

Nice, thanks Chris!

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.

[query] StreamConstraintsException in Spark backend

4 participants