loopbackio / loopback-connector-cassandra

Cassandra connector for the LoopBack framework.
Other
13 stars 22 forks source link

Add support to composite keys for updateAttributes #62

Closed shenghu closed 6 years ago

shenghu commented 6 years ago

Description

When using updateAttributes and the passed data contains primary columns (including partition keys and clustering keys), it failed w/ the error below. (See related issue for details).

Uncaught AssertionError: expected ResponseError { name: 'ResponseError', info: 'Represents an error message from the server', message: 'PRIMARY KEY part str found in SET part', code: 8704, query: 'UPDATE "CASS_SORTABLE" SET "str"=?,"num"=?,"yearMonth"=? WHERE "patStr"=?' } to not exist

Related issues

Checklist

shenghu commented 6 years ago

@jannyHou @dhmlau can you please review this change?

shenghu commented 6 years ago

@jannyHou hey, can you please review this?

shenghu commented 6 years ago

@slnode test please

shenghu commented 6 years ago

@bajtos @raymondfeng how does loopback-datasource-juggler support composite primary key?

shenghu commented 6 years ago

This pr now depends on https://github.com/strongloop/loopback-connector-cassandra/pull/66

shenghu commented 6 years ago

@slnode please test

bajtos commented 6 years ago

how does loopback-datasource-juggler support composite primary key?

I think composite keys are a feature that may work in theory but was never validated in practice, at least AFAIK. I would be surprised if composite keys were working out of the box as they are implemented now, especially when it comes to the REST API provided by LoopBack models.

shenghu commented 6 years ago

@bajtos composite keys are common in cassandra. Normally a partition key plus clustering keys. And clustering key can not be updated. This PR just avoid setting clustering key (and partition key) in update, and move the clustering key into where. Full support of composite keys may need change in juggler.

I found getId() doesn't return multiple keys which might be the root cause.

dhmlau commented 6 years ago

@bajtos @raymondfeng , any comments on how to go about this PR?

raymondfeng commented 6 years ago

The current juggler does not support composite key for *ById operations. I'm fine with the PR.

dhmlau commented 6 years ago

@slnode test please

shenghu commented 6 years ago

@raymondfeng do you have suggestion how to make juggler support composite primary keys? I think it is common requirement for relational db. And we probably need it on cassandra soon. Thanks for review.

shenghu commented 6 years ago

@dhmlau please let me know whether this can proceed and be merged.

raymondfeng commented 6 years ago

@shenghu I approve it. Thanks!

dhmlau commented 6 years ago

Thanks @shenghu for your contribution. And sorry about the delay. Your PR has landed.