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

Issues 1921, 1923, and 2499 - whole-system search updates and related changes #2519

Merged
merged 34 commits into from
Jun 25, 2021

Conversation

prb112
Copy link
Contributor

@prb112 prb112 commented Jun 16, 2021

Addresses #1921, #1923, and #2499.

Also includes schema updates associated with #2195 and partially addresses #2348 by introducing ParameterTableSupport

Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
…anonical url parsing

Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
…param tables

Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
@lmsurpre lmsurpre changed the title Feature Branch Issues 1921, 1923, and 2499 - whole-system search updates and related changes Jun 22, 2021
@lmsurpre
Copy link
Member

deprecates UriParmVal.java

updateStatement.setLong(3, logicalResourceId);
updateStatement.setInt(4, versionId);
updateStatement.setInt(3, versionId);
updateStatement.setLong(4, logicalResourceId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good catch but what was the result of the previous code? I guess InitializeLogicalResourceDenorms.java was just broken for Derby?

Copy link
Collaborator

@punktilious punktilious Jun 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes - I expect it would fail with FK violations

michaelwschroeder and others added 7 commits June 23, 2021 07:25
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Issue #1922 - implement whole-system search in new query builder
* Get the set of all resource type names.
* @return
*/
Set<String> getResourceTypeNames() throws FHIRPersistenceException;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ModelSupport we have a similar method which takes an argument for whether or not you want it to include "abstract" types.
At the very least I think we should clearly document whether this list will include those types or not (seems like yes at the moment, but should it?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should fix this when we properly implement support for a subset of resources (so that the whole-system search union only selects from the tables which are actually provisioned in the schema).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added as a comment to #819 (comment) but I still think we should update the javadoc to indicate that it current returns abstract types in the meantime.

punktilious and others added 12 commits June 23, 2021 15:41
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Mike Schroeder <mschroed@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
…f parameter codes

Signed-off-by: Robin Arnold <robin.arnold23@ibm.com>
Issue #1922 - don't run reindex tests during search tests
Copy link
Contributor

@michaelwschroeder michaelwschroeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just some minor documentation comments

@lmsurpre lmsurpre changed the title Issues 1921, 1923, and 2499 - whole-system search updates and related changes Issues 1921, 1923, 2348, and 2499 - whole-system search updates and related changes Jun 25, 2021
@lmsurpre lmsurpre self-requested a review June 25, 2021 17:36
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@punktilious punktilious merged commit 583b798 into main Jun 25, 2021
@punktilious punktilious deleted the robin-perf-eval branch June 25, 2021 18:44
@lmsurpre lmsurpre changed the title Issues 1921, 1923, 2348, and 2499 - whole-system search updates and related changes Issues 1921, 1923, and 2499 - whole-system search updates and related changes Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants