-
Notifications
You must be signed in to change notification settings - Fork 237
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
Introduce a new API to update provided query fields and update query protocol to record originalVersionId #2633
Introduce a new API to update provided query fields and update query protocol to record originalVersionId #2633
Conversation
e6a5fa7
to
ae6240a
Compare
...ion-query/src/main/java/org/finos/legend/engine/application/query/api/QueryStoreManager.java
Outdated
Show resolved
Hide resolved
...query/src/test/java/org/finos/legend/engine/application/query/api/TestQueryStoreManager.java
Outdated
Show resolved
Hide resolved
...tion-query/src/main/java/org/finos/legend/engine/application/query/api/ApplicationQuery.java
Outdated
Show resolved
Hide resolved
74b60af
to
3bb1e56
Compare
{ | ||
field.setAccessible(true); | ||
Object updatedValue = field.get(updatedQuery); | ||
if (updatedValue != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might require some rethinking about if we have the api take from Query updatedQuery
to something ese
but one use case this current flow does not support is if we want to set a value to null if it has already been set to something.
woud using the mongo set operation help here, and update only those fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the patchQuery function still takes in a parameter of type Query that we are going to update fields upon, to generate the payload of mongo set operation, I think we'll still need to compose a document by iterating the provided parameter to get non-null fields, then call set? 😅😅
Or, could you please be more specific about what the parameter type should be? 👀😅😅
Thanks
… to record originalVersionId
37d0f9c
to
932e62b
Compare
What type of PR is this?
improvement
What does this PR do / why is it needed ?
Introduce a new API to update query version and update query protocol to record originalVersionId
Which issue(s) this PR fixes:
Fixes #
Other notes for reviewers:
Does this PR introduce a user-facing change?