Skip to content

Fb statementdataiterator strict #6421

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Fb statementdataiterator strict #6421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2d28e3
Fixes for dataset table aliases
labkey-matthewb Mar 6, 2025
6caeab5
DatasetUpdateService.TestCase
labkey-matthewb Mar 6, 2025
e4ec9bd
Merge remote-tracking branch 'origin/24.11_fb_dataset_aliases' into f…
labkey-matthewb Mar 6, 2025
0d7ad38
Add a strict column mapping check to StatementDataIterator
labkey-matthewb Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions api/src/org/labkey/api/data/SchemaColumnMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public class SchemaColumnMetaData
{
private final SchemaTableInfo _tinfo;
private final List<ColumnInfo> _columns = new ArrayList<>();
private AliasManager _aliasManager;

private Map<String, ColumnInfo> _colMap = null;
private @NotNull List<String> _pkColumnNames = new ArrayList<>();
Expand Down Expand Up @@ -89,14 +90,14 @@ public SchemaColumnMetaData(SchemaTableInfo tinfo, List<MutableColumnInfo> cols,
loadColumnsFromXml(_tinfo, xmlTable);
}

private AliasManager getAliasManager(AliasManager aliasManager)
private AliasManager getAliasManager()
{
if (null == aliasManager)
if (null == _aliasManager)
{
aliasManager = new AliasManager(_tinfo.getSchema());
aliasManager.claimAliases(_columns);
_aliasManager = new AliasManager(_tinfo.getSchema());
_aliasManager.claimAliases(_columns);
}
return aliasManager;
return _aliasManager;
}

private void loadColumnsFromXml(SchemaTableInfo tinfo, TableType xmlTable)
Expand Down Expand Up @@ -153,8 +154,7 @@ private void loadColumnsFromXml(SchemaTableInfo tinfo, TableType xmlTable)
colInfo = new BaseColumnInfo(FieldKey.fromParts(xmlColumn.getColumnName()), tinfo);
colInfo.setNullable(true);
loadFromXml(xmlColumn, colInfo, false);
aliasManager = getAliasManager(aliasManager);
aliasManager.ensureAlias(colInfo);
getAliasManager().ensureAlias(colInfo);
addColumn(colInfo);
}
}
Expand Down Expand Up @@ -198,8 +198,7 @@ else if (xmlColumn.isSetWrappedColumnName() && isNotBlank(xmlColumn.getWrappedCo
exprColumn.getAlias();
assert exprColumn.isAliasSet();
// now reserve that alias
aliasManager = getAliasManager(aliasManager);
aliasManager.ensureAlias(exprColumn);
getAliasManager().ensureAlias(exprColumn);
addColumn(exprColumn);
}
catch (QueryParseException qpe)
Expand Down Expand Up @@ -422,9 +421,14 @@ protected void addColumn(MutableColumnInfo column)
if (!column.isAliasSet())
{
if (null != column.getMetaDataName())
{
column.setAlias(column.getMetaDataName());
getAliasManager().claimAlias(column);
}
else
column.setAlias(column.getName());
{
getAliasManager().ensureAlias(column);
}
}
_colMap = null;
}
Expand Down
14 changes: 10 additions & 4 deletions api/src/org/labkey/api/dataiterator/StatementDataIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down Expand Up @@ -190,17 +191,22 @@ void init()
ParameterMapStatement stmt = _stmts[set];
// map from source to target
ArrayList<Triple> bindings = new ArrayList<>(stmt.size());
IdentityHashMap<Parameter,ColumnInfo> boundParametersMap = new IdentityHashMap<>();
// by name
for (int i=1 ; i<=_data.getColumnCount() ; i++)
{
ColumnInfo col = _data.getColumnInfo(i);
Parameter to = null;
if (null != col.getPropertyURI())
Parameter to = stmt.getParameter(col.getName());
if (null == to && null != col.getPropertyURI())
to = stmt.getParameter(col.getPropertyURI());
if (to == null)
to = stmt.getParameter(col.getName());
if (null != to)
{
var prev = boundParametersMap.put(to,col);
if (null != prev)
{
throw new IllegalStateException("Two columns mapped to target column '" + to.getName() + "'." +
" Found '" + prev.getName() + "' and '" + col.getName() + "'.");
}
FieldKey mvName = col.getMvColumnName();
bindings.add(new Triple(_data.getSupplier(i), to,
(null != mvName ? getMvParameter(stmt, mvName) : null)));
Expand Down
2 changes: 2 additions & 0 deletions api/src/org/labkey/api/query/AliasedColumn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public AliasedColumn(TableInfo parent, FieldKey key, ColumnInfo column, boolean
{
super(key, parent);
copyAttributesFrom(column);
// property URI should be unique, and certainly within one table
setPropertyURI(null);

Map<FieldKey, FieldKey> remap = new HashMap<>();
remap.put(column.getFieldKey(), key);
Expand Down
4 changes: 3 additions & 1 deletion study/src/org/labkey/study/StudyModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
import org.labkey.study.qc.StudyQCImportExportHelper;
import org.labkey.study.qc.StudyQCStateHandler;
import org.labkey.study.query.DatasetQueryView;
import org.labkey.study.query.DatasetUpdateService;
import org.labkey.study.query.QueryDatasetQueryChangeListener;
import org.labkey.study.query.StudyPersonnelDomainKind;
import org.labkey.study.query.StudyQuerySchema;
Expand Down Expand Up @@ -777,7 +778,8 @@ public Set<Class> getIntegrationTests()
StudyManager.VisitCreationTestCase.class,
StudyModule.TestCase.class,
TreatmentManager.TreatmentDataTestCase.class,
VisitImpl.TestCase.class
VisitImpl.TestCase.class,
DatasetUpdateService.TestCase.class
);
}

Expand Down
35 changes: 20 additions & 15 deletions study/src/org/labkey/study/model/DatasetDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,9 @@ public TableInfo getSchemaTableInfo()
public CaseInsensitiveHashMap<String> remapSchemaColumns()
{
CaseInsensitiveHashMap<String> m = new CaseInsensitiveHashMap<>();

if (!DatasetDomainKind.PARTICIPANTID.equalsIgnoreCase(_study.getSubjectColumnName()))
m.put(DatasetDomainKind.PARTICIPANTID, _study.getSubjectColumnName());

// why did I add an underscore to the stored mv indicators???
for (ColumnInfo col : getColumns())
Expand Down Expand Up @@ -2754,38 +2757,40 @@ public List<Map<String, Object>> getDatasetRows(User u, Collection<String> lsids
((ArrayListMap)datas.get(0)).getFindMap().remove("_key");
}

List<Map<String, Object>> canonicalDatas = new ArrayList<>(datas.size());
// results should not be sensitive to the column aliases, convert aliases to column names
CaseInsensitiveHashMap<String> aliasToColumnNames = new CaseInsensitiveHashMap<>();
List<ColumnInfo> columns = tInfo.getColumns();
for (ColumnInfo col : columns)
{
var name = col.getName();
// NOTE: case of columns in tInfo and queryTableInfo seem to match except for QCstate, leaving for now
var queryCol = queryTableInfo.getColumn(name);
if (null != queryCol)
name = queryCol.getName();
aliasToColumnNames.put(col.getAlias(), name);
}

List<Map<String, Object>> canonicalDatas = new ArrayList<>(datas.size());
for (Map<String, Object> data : datas)
{
canonicalDatas.add(canonicalizeDatasetRow(data, queryTableInfo.getColumns()));
canonicalDatas.add(canonicalizeDatasetRow(data, aliasToColumnNames));
}

return canonicalDatas;
}

// change a map's keys to have proper casing just like the list of columns
private Map<String,Object> canonicalizeDatasetRow(Map<String,Object> source, List<ColumnInfo> columns)
private Map<String,Object> canonicalizeDatasetRow(Map<String,Object> source, Map<String,String> aliasToColumnNames)
{
CaseInsensitiveHashMap<String> keyNames = new CaseInsensitiveHashMap<>();
for (ColumnInfo col : columns)
{
keyNames.put(col.getName(), col.getName());
}

Map<String,Object> result = new CaseInsensitiveHashMap<>();

for (Map.Entry<String,Object> entry : source.entrySet())
{
String key = entry.getKey();
String newKey = keyNames.get(key);
if (newKey != null)
key = newKey;
else if ("_row".equals(key))
if ("_row".equals(key))
continue;
key = aliasToColumnNames.getOrDefault(key, key);
result.put(key, entry.getValue());
}

return result;
}

Expand Down
Loading