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

INSERT INTO and UPDATE SET fail with ',' or '=' contained in Values #85

Open
SerethiX opened this issue Dec 5, 2018 · 0 comments
Open

Comments

@SerethiX
Copy link
Contributor

SerethiX commented Dec 5, 2018

What is this feature about (expected vs actual behaviour)?

  1. Inserting Values that contain , result in an error when building the parameter key value array
  • Expected to be able to insert any string
  1. Update Statements that contain = as or in one of their values fail aswell, they truncate the value
  • Expected to be able to update values with any strin without truncating

How can I reproduce it?

Execute

  1. Execute INSERT INTO foo (bar) VALUES ('foo, bar')
  2. Execute UPDATE foo SET bar = 'x=2'

using this Driver. You will get a warning, that array_combine has different element counts between the two arrays and an error when providing the false return value of array_combine into the JsonFormatter, which expects an array on the encodemethod.

Does it take minutes, hours or days to fix?

It took hours.

Any additional information?

Wrote a simple lexer that detects if the current value is a string (based on changes introduced in #77 ; looks out for '' "" `` to start and end a string), appending everything until the next , after a string ended to the current value, then starting a new one.

Escaping is handled like PDO does it by adding another char of the same type in front of it in SqlQuery::quote(). The lexer then just toggles stringMode off and on again.

The dequoting then takes place in Value::create()

Edited the Tests a little for some more edged cases like single Quotes around a text etc. Also a few more Tests got modified to test for the exact same problems, currently no problems found.

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 a pull request may close this issue.

1 participant