Handle object retrieval with composite primary keys #426
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the current implementation,
SQLAchemyBase.resolve_id
uses simple string conversion to convert object primary keys into string format, andSQLAlchemyBase.get_node
passes the obtained key to a SQLAlchemy session to retrieve the object. However, this does not work for composite primary keys, as the string would first need to be converted back to a tuple. While this can be done witheval
, this would be a security hole, since it would allow execution of arbitrary code, and ids are normally received as untrusted output.The simplest and relatively general solution for the above would be to use the
json
module to serialize and deserialize the keys. However, not all allowed primary keys are json-serializable (e.g.DATETIME
), so additional functionality may be required for a general case.Instead of trying to provide a general solution for the above, the patch abstracts serialization of primary keys to a class attribute that can be configured based on the needs of the SQLAlchemy schema on a per-table basis. Schemas that do not contain tables with composite keys can continue to use the current implementation, which is provided as default.
As an example, to handle classes with json-serializable composite primary keys, the following configuration would be necessary:
I am open to suggestions on code restructuring, and will add tests to exercise the functionality.