Add InventoryFilter. Clear orphaned issues. #7165
Conversation
labkey-adam
changed the title
| // Add a filter to select all rows where the object property with <propertyId> equals the filter value | ||
| protected void addObjectPropertyFilter(OrClause orClause, DataFilter filter, TableInfo sourceTable, FieldKey fKey, int propertyId) | ||
| { | ||
| SQLFragment flagWhere = new SQLFragment("lsid IN (SELECT ObjectURI FROM exp.Object WHERE ObjectId IN (SELECT ObjectId FROM exp.ObjectProperty WHERE StringValue = ? AND PropertyId = ?))", filter.condition().getParamVals()[0], propertyId); |
There was a problem hiding this comment.
Since most exp tables have an object it, it might be worth having a parameter that indicates whether to use "lsid" or "objectid"
There was a problem hiding this comment.
This runs the risk of the database actually evaluating ((SELECT ObjectURI FROM exp.Object WHERE ObjectId IN (SELECT ObjectId FROM exp.ObjectProperty WHERE StringValue = ? AND PropertyId = ?)) in a somewhat expensive way. It's index are optimized for objectid=? and propertyid=? joins.
Iff there is a performance issue this might be worth trying
lsid:: WHERE ? = (SELECT StringValue FROM (O WHERE objecturi=lsid) JOIN OP ON objectid WHERE PropertyId=?)
or
objectid:: WHERE ? = (SELECT StringValue FROM OP WHERE OP.ObjectId=.objectid PropertyId=?)
There was a problem hiding this comment.
Since most exp tables have an object it, it might be worth having a parameter that indicates whether to use "lsid" or "objectid"
This is called to filter provisioned tables, none of which have ObjectId (that I've seen). Sample Type tables do have RowId (a material RowId) and that handler generates a custom join that takes advantage. I don't think we can generalize this method to handle all the ways we join into exp.ObjectProperty... in my current FB, I've improved the comments to make it clear this is for cases where LSID is the only option.
There was a problem hiding this comment.
This runs the risk of the database actually evaluating ((SELECT ObjectURI FROM exp.Object WHERE ObjectId IN (SELECT ObjectId FROM exp.ObjectProperty WHERE StringValue = ? AND PropertyId = ?)) in a somewhat expensive way. It's index are optimized for objectid=? and propertyid=? joins. Iff there is a performance issue this might be worth trying
lsid:: WHERE ? = (SELECT StringValue FROM (O WHERE objecturi=lsid) JOIN OP ON objectid WHERE PropertyId=?) or objectid:: WHERE ? = (SELECT StringValue FROM OP WHERE OP.ObjectId=.objectid PropertyId=?)
@labkey-matthewb Here's how I've rewritten the full data class provisioned table SELECT statement (with hard-coded values instead of placeholders). Any further suggestions? (Note that because we're constructing and passing around FilterClauses, the first join has to be a sub-select.):
SELECT * FROM expdataclass.c1563d4300_protsequence dc WHERE lsid IN (SELECT ObjectURI FROM exp.Object o INNER JOIN exp.ObjectProperty op ON o.ObjectId = op.ObjectId WHERE StringValue = 'J.POD2' AND PropertyId = 70)
There was a problem hiding this comment.
I see. That makes sense. Simpler is usually better (or not worse) so the rewrite “looks” better to me (does that count as vibe coding)
There was a problem hiding this comment.
Or maybe you're saying this is better:
SELECT * FROM expdataclass.c1563d4300_protsequence dc WHERE 'J.POD2' = (
SELECT StringValue FROM exp.ObjectProperty op INNER JOIN exp.Object o ON op.ObjectId = o.ObjectId AND ObjectURI = dc.LSID WHERE PropertyId = 70
)
There was a problem hiding this comment.
That’s how I would have written it by hand as a first guess (that kinda mirrors what we do for PropertyColumn), but if you have to stuff it into a filter clause, I can see why your rewrite makes sense. I would just keep these ideas in your back pocket if the performance is a problem.
|
|
||
| // Helper method that parses a data filter then adds it and its container to the provided collections, coalescing | ||
| // cases where multiple containers specify the same filter | ||
| static void addDataFilter(String filterName, List<DataFilter> dataFilters, Set<GUID> filteredContainers, GUID guid, String filter) |
There was a problem hiding this comment.
Too bad filter isn't a FilterClause (but I don't know how we get to this point)
labkey-matthewb
left a comment
labkey-matthewb
left a comment
There was a problem hiding this comment.
I'm curious about what metadata/conventions we could add to our schemas to simplify this work.
2 participants
Rationale
Refactor domain filtering to allow code sharing between tables with automatic Flag columns and extensible tables (like inventory Location, Box, and Item).
Clean up orphaned issues rows.