Skip to content

Fix: Symbols in binds#119

Merged
TorstenDittmann merged 12 commits intomainfrom
fix-symbols-in-binds
May 12, 2022
Merged

Fix: Symbols in binds#119
TorstenDittmann merged 12 commits intomainfrom
fix-symbols-in-binds

Conversation

@Meldiron
Copy link
Copy Markdown
Contributor

@Meldiron Meldiron commented Apr 1, 2022

🚨 Breaking Change: Dot . is no longer supported (key, collection, attribute, id...) 🚨

Currently, if you create an attribute with a dash or dot my-name or my-name, the attribute is created. Problem is, if you try to createDocument or updateDocument, you get SQL query error:

CleanShot 2022-04-01 at 11 35 23

After this PR, binds are custom made and do not include attribute symbols. This resolved the problem and dashes/dots now work properly:

CleanShot 2022-04-01 at 11 35 29

  • Manual QA
  • Write tests

Issues:

@Meldiron
Copy link
Copy Markdown
Contributor Author

Meldiron commented Apr 1, 2022

Mongo tests are currently failing. The reason is, we try to support . in the attribute name, but Mongo does some nesting magic if you provide dot in attribute name: https://stackoverflow.com/questions/12397118/mongodb-dot-in-key-name

I will need more feedback to decide what to do in this scenario. Do we want to drop support for dot in attribute name because of Mongo?

CleanShot 2022-04-01 at 12 40 53

Comment thread src/Database/Adapter/MariaDB.php Outdated
Comment thread README.md
Comment thread src/Database/Document.php Outdated
Comment thread src/Database/Adapter/MariaDB.php Outdated
@Meldiron
Copy link
Copy Markdown
Contributor Author

@TorstenDittmann Filter allows - symbol, it's PDO who doesnt like the symbol.

@TorstenDittmann
Copy link
Copy Markdown
Contributor

@TorstenDittmann Filter allows - symbol, it's PDO who doesnt like the symbol.

That's fine, but we don't need a second array $bindmap to achieve this 👍🏻

@Meldiron
Copy link
Copy Markdown
Contributor Author

Meldiron commented May 4, 2022

@TorstenDittmann Addressed. We now use only index, no map variable required.

@TorstenDittmann TorstenDittmann merged commit ab30b2a into main May 12, 2022
@TorstenDittmann TorstenDittmann deleted the fix-symbols-in-binds branch May 12, 2022 17:06
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.

4 participants