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

Fixes a bug where you cannot use a runtime operator in a SQL query #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iainmckay
Copy link
Contributor

@iainmckay iainmckay commented May 26, 2017

This attempts to fix the bug documented in #40. As you stated, it wasn't a trivial fix and our fix is, unfortunately, a bit disruptive.

This introduces a few new objects to track what type of operators are being created so that they can be formatted correctly when they are written out to a cache. Breaking out of strings is a bit messy but it was the most straightforward method we could find to solve the issue.

We've only confirmed that the native and DoctrineORM targets work right now as that is what we use in our project. I'd like feedback on our approach but we do intend on fixing the other targets and tests to work.

@K-Phoen
Copy link
Owner

K-Phoen commented Jun 2, 2017

I just drop a quick comment to say that I saw your PR and will definitely review it, but that I won't be able to do it before at least a couple of weeks (due to a lack of time).

Thanks for the PR and sorry for the inconvenience :|

@iainmckay
Copy link
Contributor Author

Sure. It's not in a state that can be merged it was more to see if you agreed with the general approach then we'll wrap it up.

@iainmckay
Copy link
Contributor Author

Do you have any feedback on this?

@iainmckay
Copy link
Contributor Author

Ping

@iainmckay iainmckay force-pushed the runtime-sql-operator branch 3 times, most recently from a345f6f to f9bbb47 Compare January 9, 2018 14:27
@iainmckay iainmckay force-pushed the runtime-sql-operator branch 2 times, most recently from f822b93 to 61bc703 Compare March 8, 2018 09: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.

2 participants