Support field type conversion for Multi Choice fields #7356

XingY · 2026-01-22T23:46:53Z

Rationale

Related Pull Requests

Changes

Copilot

Pull request overview

This pull request implements support for converting Multi Choice field types to/from other field types (primarily text). The changes include schema modifications, database migration logic, and automatic updates to custom view filters when column types change.

Changes:

Extended the QueryChangeListener interface to support column type changes with a new queryName parameter
Added PostgreSQL-specific SQL transformation logic for converting between text[] (MULTI_CHOICE) and text types
Implemented automatic filter operator translation in custom views when Multi Choice fields are converted (e.g., arraycontainsany → in)

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
api/src/org/labkey/api/query/QueryChangeListener.java	Added `ColumnType` property, filter transformation utilities, and `queryName` parameter to interface
api/src/org/labkey/api/query/AbstractQueryChangeListener.java	Updated to pass `queryName` parameter through to abstract methods
api/src/org/labkey/api/query/QueryService.java	Added `fireQueryColumnChanged` method for column-specific change notifications
api/src/org/labkey/api/data/TableChange.java	Added support for tracking old property types during column type changes
query/src/org/labkey/query/QueryServiceImpl.java	Implemented `fireQueryColumnChanged` method
query/src/org/labkey/query/QueryDefQueryChangeListener.java	Updated signature to accept `queryName` parameter
query/src/org/labkey/query/CustomViewQueryChangeListener.java	Added logic to update custom view filters when column types change
query/src/org/labkey/query/QuerySnapshotQueryChangeListener.java	Updated signature to accept `queryName` parameter
query/src/org/labkey/query/reports/ReportQueryChangeListener.java	Updated signature to accept `queryName` parameter
query/src/org/labkey/query/persist/QueryManager.java	Updated to handle `queryName` parameter in change notifications
core/src/org/labkey/core/dialect/PostgreSql92Dialect.java	Added SQL transformation logic for MULTI_CHOICE ↔ text conversions
experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java	Added column type change notification trigger
experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java	Added validation to prevent conversion when multi-value data exists
experiment/PropertyQueryChangeListener.java	Updated signature to accept `queryName` parameter
experiment/ExperimentQueryChangeListener.java	Updated signature to accept `queryName` parameter
study/src/org/labkey/study/query/QueryDatasetQueryChangeListener.java	Updated signature to accept `queryName` parameter
experiment/package.json	Updated `@labkey/components` dependency to feature branch version
experiment/package-lock.json	Updated lock file for new dependency version
core/package.json	Updated `@labkey/components` dependency to feature branch version
core/package-lock.json	Updated lock file for new dependency version

Files not reviewed (2)

core/package-lock.json: Language not supported
experiment/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T00:03:25Z

api/src/org/labkey/api/data/TableChange.java

+    public void setOldPropertyTypes(Map<String, PropertyType> oldPropTypes)
+    {
+        _oldPropTypes = oldPropTypes;
+    }


Duplicate method definition. The method setOldPropertyTypes is defined twice in this class - once at lines 339-342 as setOldPropTypes and again at lines 364-367 as setOldPropertyTypes. Remove one of these duplicate methods.

Copilot · 2026-01-23T00:03:25Z

core/src/org/labkey/core/dialect/PostgreSql92Dialect.java


        for (PropertyStorageSpec column : change.getColumns())
        {
+            PropertyType oldPropertyType = change.getOldPropTypes().get(column.getName());


Potential NullPointerException. The getOldPropTypes() method can return null (as the field _oldPropTypes is initialized to null), but this code calls .get() on the result without a null check. Add a null check before calling .get() or initialize the map to an empty map.

Suggested change

PropertyType oldPropertyType = change.getOldPropTypes().get(column.getName());

Map<String, PropertyType> oldPropTypes = change.getOldPropTypes();

PropertyType oldPropertyType = oldPropTypes != null ? oldPropTypes.get(column.getName()) : null;

Copilot · 2026-01-23T00:03:25Z

experiment/package.json

  },
  "dependencies": {
-    "@labkey/components": "7.13.0"
+    "@labkey/components": "7.14.0-fb-mvtc-convert.1"


Feature branch dependency version. The dependency uses a feature branch version 7.14.0-fb-mvtc-convert.1 which appears to be a pre-release or development version. Before merging to main, this should typically be updated to a stable release version to avoid potential dependency issues in production.

Suggested change

"@labkey/components": "7.14.0-fb-mvtc-convert.1"

"@labkey/components": "7.14.0"

Copilot · 2026-01-23T00:03:26Z

core/package.json

  },
  "dependencies": {
-    "@labkey/components": "7.13.0",
+    "@labkey/components": "7.14.0-fb-mvtc-convert.1",


Feature branch dependency version. The dependency uses a feature branch version 7.14.0-fb-mvtc-convert.1 which appears to be a pre-release or development version. Before merging to main, this should typically be updated to a stable release version to avoid potential dependency issues in production.

Suggested change

"@labkey/components": "7.14.0-fb-mvtc-convert.1",

"@labkey/components": "7.14.0",

Copilot · 2026-01-23T00:03:26Z

query/src/org/labkey/query/CustomViewQueryChangeListener.java

+                    for (String filterPart : filterComponents)
+                    {
+                        String updatedPart = QueryChangeListener.getUpdatedFilterStrOnColumnTypeUpdate(filterPart, columnName, oldDp.getPropertyType(), newDp.getPropertyType());


Missing separator between filter parts. When reconstructing the filter string, each filter part should be appended with an ampersand separator (except the last one). Currently, the code appends filter parts without any separator, which will result in malformed filter strings. Add & between filter parts when appending to the StringBuilder.

Suggested change

for (String filterPart : filterComponents)

{

String updatedPart = QueryChangeListener.getUpdatedFilterStrOnColumnTypeUpdate(filterPart, columnName, oldDp.getPropertyType(), newDp.getPropertyType());

for (int i = 0; i < filterComponents.length; i++)

{

String filterPart = filterComponents[i];

String updatedPart = QueryChangeListener.getUpdatedFilterStrOnColumnTypeUpdate(filterPart, columnName, oldDp.getPropertyType(), newDp.getPropertyType());

if (i > 0)

updatedFilterAndSort.append("&");

Copilot · 2026-01-23T00:03:26Z

experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java

+                        String sql = "SELECT COUNT(*) FROM " + kind.getStorageSchemaName() + "." + domain.getStorageTableName() +
+                                " WHERE " + prop.getPropertyDescriptor().getStorageColumnName() + " IS NOT NULL AND " +
+                                " array_length(" + prop.getPropertyDescriptor().getStorageColumnName() + ", 1) > 1";


SQL injection risk and poor SQL construction. The SQL query is being constructed using string concatenation instead of using SQLFragment properly. While storage column names are internally generated, this is still a poor practice that could lead to SQL injection vulnerabilities. Use SQLFragment's methods like appendIdentifier() for table and column names to ensure proper escaping and safety.

Suggested change

String sql = "SELECT COUNT(*) FROM " + kind.getStorageSchemaName() + "." + domain.getStorageTableName() +

" WHERE " + prop.getPropertyDescriptor().getStorageColumnName() + " IS NOT NULL AND " +

" array_length(" + prop.getPropertyDescriptor().getStorageColumnName() + ", 1) > 1";

SQLFragment sql = new SQLFragment();

sql.append("SELECT COUNT(*) FROM ");

sql.appendIdentifier(kind.getStorageSchemaName());

sql.append(".");

sql.appendIdentifier(domain.getStorageTableName());

sql.append(" WHERE ");

sql.appendIdentifier(prop.getPropertyDescriptor().getStorageColumnName());

sql.append(" IS NOT NULL AND array_length(");

sql.appendIdentifier(prop.getPropertyDescriptor().getStorageColumnName());

sql.append(", 1) > 1");

Copilot · 2026-01-23T00:03:26Z

api/src/org/labkey/api/query/QueryChangeListener.java

+            // Replace all occurrences of %2C with %2C%20,
+            // "," -> ", " during array to string conversion
+            return updated.replace("%2C", "%2C%20");
+        }
+        if (containsOp(updated, columnName, "arraynotmatches"))
+        {
+            updated = replaceOp(updated, columnName, "arraynotmatches", "neq");
+            // Replace all occurrences of %2C with %2C%20
+            return updated.replace("%2C", "%2C%20");
+        }
+        if (containsOp(updated, columnName, "arraycontainsany"))
+        {
+            updated = replaceOp(updated, columnName, "arraycontainsany", "in");
+            // Replace all occurrences of %2C with %3B
+            // ";" is used as the separator for "in" operator
+            return updated.replace("%2C", "%3B");
+        }
+        if (containsOp(updated, columnName, "arraycontainsnone"))
+        {
+            updated = replaceOp(updated, columnName, "arraycontainsnone", "notin");
+            // Replace all occurrences of %2C with %3B
+            // ";" is used as the separator for "notin" operator
+            return updated.replace("%2C", "%3B");
+        }
+
+        // No matching operator found for this column, drop the filter
+        return "";
+    }
+


Potential data corruption in filter values. The .replace() method replaces all occurrences globally in the filter string, which could inadvertently modify URL-encoded characters within filter values themselves, not just separators. For example, if a filter value contains %2C (comma) or %3B (semicolon) as part of the actual data, these would also be replaced. Consider using a more targeted approach that only replaces separators in the filter value portion, or document that this limitation is acceptable for the Multi Choice field conversion use case.

Suggested change

// Replace all occurrences of %2C with %2C%20,

// "," -> ", " during array to string conversion

return updated.replace("%2C", "%2C%20");

}

if (containsOp(updated, columnName, "arraynotmatches"))

{

updated = replaceOp(updated, columnName, "arraynotmatches", "neq");

// Replace all occurrences of %2C with %2C%20

return updated.replace("%2C", "%2C%20");

}

if (containsOp(updated, columnName, "arraycontainsany"))

{

updated = replaceOp(updated, columnName, "arraycontainsany", "in");

// Replace all occurrences of %2C with %3B

// ";" is used as the separator for "in" operator

return updated.replace("%2C", "%3B");

}

if (containsOp(updated, columnName, "arraycontainsnone"))

{

updated = replaceOp(updated, columnName, "arraycontainsnone", "notin");

// Replace all occurrences of %2C with %3B

// ";" is used as the separator for "notin" operator

return updated.replace("%2C", "%3B");

}

// No matching operator found for this column, drop the filter

return "";

}

// Replace "%2C" separators with "%2C%20" (", " during array to string conversion)

// Only update the value portion of this column's filter

updated = replaceSeparatorsInFilterValue(updated, columnName, "eq", "%2C", "%2C%20");

return updated;

}

if (containsOp(updated, columnName, "arraynotmatches"))

{

updated = replaceOp(updated, columnName, "arraynotmatches", "neq");

// Replace "%2C" separators with "%2C%20" for the value portion of this column's filter

updated = replaceSeparatorsInFilterValue(updated, columnName, "neq", "%2C", "%2C%20");

return updated;

}

if (containsOp(updated, columnName, "arraycontainsany"))

{

updated = replaceOp(updated, columnName, "arraycontainsany", "in");

// Replace "%2C" separators with "%3B" (";" is used as the separator for the "in" operator)

updated = replaceSeparatorsInFilterValue(updated, columnName, "in", "%2C", "%3B");

return updated;

}

if (containsOp(updated, columnName, "arraycontainsnone"))

{

updated = replaceOp(updated, columnName, "arraycontainsnone", "notin");

// Replace "%2C" separators with "%3B" (";" is used as the separator for the "notin" operator)

updated = replaceSeparatorsInFilterValue(updated, columnName, "notin", "%2C", "%3B");

return updated;

}

// No matching operator found for this column, drop the filter

return "";

}

private static String replaceSeparatorsInFilterValue(String filterStr, String columnName, String operator, String from, String to)

{

if (filterStr == null || columnName == null || operator == null || from == null || to == null)

return filterStr;

// Build a case-insensitive pattern to match: filter.<columnName>~<operator>=<value...>

String patternStr = "(?i)(filter\\." + Pattern.quote(columnName) + "~" + Pattern.quote(operator) + "=)([^&]*)";

Pattern pattern = Pattern.compile(patternStr);

Matcher matcher = pattern.matcher(filterStr);

if (!matcher.find())

return filterStr;

String prefix = matcher.group(1);

String value = matcher.group(2);

String newValue = value.replace(from, to);

// Reconstruct the filter string with the updated value portion

return matcher.replaceFirst(Matcher.quoteReplacement(prefix + newValue));

}

Copilot · 2026-01-23T00:03:27Z

api/src/org/labkey/api/query/QueryChangeListener.java

+        if (containsOp(updated, columnName, "in"))
+        {
+            updated = replaceOp(updated, columnName, "in", "arraycontainsany");
+            // update ";" to "," for separator appropriate for array operator
+            return updated.replace("%3B", "%2C");
+        }
+        if (containsOp(updated, columnName, "notin"))
+        {
+            updated = replaceOp(updated, columnName, "notin", "arraycontainsnone");
+            return updated.replace("%3B", "%2C");
+        }


Potential data corruption in filter values. The .replace() method replaces all occurrences globally in the filter string, which could inadvertently modify URL-encoded characters within filter values themselves, not just separators. For example, if a filter value contains %2C (comma) or %3B (semicolon) as part of the actual data, these would also be replaced. Consider using a more targeted approach that only replaces separators in the filter value portion, or document that this limitation is acceptable for the Multi Choice field conversion use case.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

XingY added 2 commits January 22, 2026 15:43

Support field type conversion for Multi Choice fields

a066d53

crlf

fc22b72

XingY requested a review from Copilot January 22, 2026 23:52

Copilot started reviewing on behalf of XingY January 22, 2026 23:52 View session

XingY mentioned this pull request Jan 22, 2026

Support field type conversion for Multi Choice fields LabKey/labkey-ui-components#1927

Open

Copilot AI reviewed Jan 23, 2026

View reviewed changes

Port Is empty filter changes from fb_mvtc_empty branch

cbb199b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support field type conversion for Multi Choice fields #7356

Support field type conversion for Multi Choice fields #7356

Uh oh!

XingY commented Jan 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	PropertyType oldPropertyType = change.getOldPropTypes().get(column.getName());
	Map<String, PropertyType> oldPropTypes = change.getOldPropTypes();
	PropertyType oldPropertyType = oldPropTypes != null ? oldPropTypes.get(column.getName()) : null;

	"@labkey/components": "7.14.0-fb-mvtc-convert.1"
	"@labkey/components": "7.14.0"

	"@labkey/components": "7.14.0-fb-mvtc-convert.1",
	"@labkey/components": "7.14.0",

-                    for (String filterPart : filterComponents)
-                    {
-                        String updatedPart = QueryChangeListener.getUpdatedFilterStrOnColumnTypeUpdate(filterPart, columnName, oldDp.getPropertyType(), newDp.getPropertyType());
+                    for (int i = 0; i < filterComponents.length; i++)
+                    {
+                        String filterPart = filterComponents[i];
+                        String updatedPart = QueryChangeListener.getUpdatedFilterStrOnColumnTypeUpdate(filterPart, columnName, oldDp.getPropertyType(), newDp.getPropertyType());
+                        if (i > 0)
+                            updatedFilterAndSort.append("&");

-                        String sql = "SELECT COUNT(*) FROM " + kind.getStorageSchemaName() + "." + domain.getStorageTableName() +
-                                " WHERE " + prop.getPropertyDescriptor().getStorageColumnName() + " IS NOT NULL AND " +
-                                " array_length(" + prop.getPropertyDescriptor().getStorageColumnName() + ", 1) > 1";
+                        SQLFragment sql = new SQLFragment();
+                        sql.append("SELECT COUNT(*) FROM ");
+                        sql.appendIdentifier(kind.getStorageSchemaName());
+                        sql.append(".");
+                        sql.appendIdentifier(domain.getStorageTableName());
+                        sql.append(" WHERE ");
+                        sql.appendIdentifier(prop.getPropertyDescriptor().getStorageColumnName());
+                        sql.append(" IS NOT NULL AND array_length(");
+                        sql.appendIdentifier(prop.getPropertyDescriptor().getStorageColumnName());
+                        sql.append(", 1) > 1");

Support field type conversion for Multi Choice fields #7356

Are you sure you want to change the base?

Support field type conversion for Multi Choice fields #7356

Uh oh!

Conversation

XingY commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Related Pull Requests

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

XingY commented Jan 22, 2026 •

edited

Loading