Only wrap ehr_lookups tables whose pseudo-PK differs from the real PK#1168
Only wrap ehr_lookups tables whose pseudo-PK differs from the real PK#1168labkey-martyp wants to merge 2 commits into
Conversation
The generic fallback in EHRLookupsUserSchema.createTable() wrapped any hard table with a single non-rowid key field and a container column in a ContainerScopedTable. For tables whose user-facing key is the true DB PK (clinpath_status, project_types, full_snomed, flag_values), the PK constraint already enforces uniqueness and the wrapper made the key column hidden and non-insertable in the UI. The heuristic now compares the promoted key against the real PK from JDBC metadata, so those tables get CustomPermissionsTable while all 26 rowid-plus-pseudo-PK tables keep container scoping. Claude-Session: https://claude.ai/code/session_013WM8RcTJDaaopwMTK4H9KX
| // and container-scoping such a table made its key column non-insertable in the UI. | ||
| String pkColName = getPkColName(ti); | ||
| if (pkColName != null && !"rowid".equalsIgnoreCase(pkColName) && ti.getColumn("container") != null) | ||
| List<String> realPk = _dbSchema.getTable(name).getPkColumnNames(); |
There was a problem hiding this comment.
My only question is whether this adds overhead. I suspect that LK might do enough TableInfo caching that this doesnt matter. Second question is whether the TableInfo could be interrogated directly (since it probably stores a reference to the schema table info), rather than calling _dbSchema.getTable(), which would ensure we dont add the overhead of re-querying the DbSchema for that TableInfo.
If you did your own investigation and dont think these issues are worth it, I dont have that strong of an opinion here.
There was a problem hiding this comment.
Yeah, good point on getting the realPk from the existing ti realTable. That change is pushed.
And to your first point, there's nothing here that will re-query the tableinfo metadata. That will already be in the SchemaTableInfoCache. So should be negligible impact on performance.
Addresses PR feedback: the wrapped SimpleTable already holds the source SchemaTableInfo, so read the real PK via getRealTable() (the same idiom ContainerScopedTable.init() uses) rather than resolving the table again through _dbSchema.getTable(). Claude-Session: https://claude.ai/code/session_01YJwkmACfqMmYiAX15YbU1D
2 participants
Rationale
This PR fixes ehr_lookups tables whose user-facing key column was hidden and non-insertable in the UI (
clinpath_status,project_types,full_snomed,flag_values). The generic fallback inEHRLookupsUserSchema.createTable()wrapped any hard table with a single non-rowid key field and a container column in aContainerScopedTable, but for tables whose user-facing key is the true DB PK the wrapper is unnecessary — the PK constraint already enforces uniqueness — and its init demoted the key column while the insert path still required a value for it.Related Pull Requests
None.
Changes
ContainerScopedTable; tables whose key is the true PK now getCustomPermissionsTable.ContainerScopedTableonly supports a single-column real PK.