mvtc empty #7348

labkey-matthewb · 2026-01-20T20:41:57Z

Rationale

For Array type required does not mean "IS NOT NULL" it means "cardinality() > 0".
Also for Multiple-choice text we prefer that null/"" converts to [] not to null.

To help implement this we consolidate column conversion into ColumnInfo.convert() and required/non-empty checking into JdbcType.isEmpty().

Related Pull Requests

Changes

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvert - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvertmake JdbcType.CHAR.convert() consistent with JdbcType.VARCHAR - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

Copilot

Pull request overview

This pull request consolidates column value conversion and required/non-empty validation logic across the codebase. The primary goals are to handle Array type validation correctly (where required means "cardinality() > 0" rather than "IS NOT NULL") and to ensure consistent conversion behavior for Multiple-choice text fields (converting null/"" to [] rather than null).

Changes:

Introduces the SimpleConvert interface to unify conversion logic across JdbcType, PropertyType, Unit, and ColumnInfo
Consolidates conversion logic into ColumnInfo.convert() instead of scattered ConvertUtils.convert() calls
Adds JdbcType.isEmpty() method to properly validate required fields based on type-specific semantics
Adds array_is_empty() SQL function for PostgreSQL to check array emptiness using cardinality

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
api/src/org/labkey/api/data/SimpleConvert.java	New interface defining conversion contract with optional performance optimization method
api/src/org/labkey/api/data/JdbcType.java	Implements SimpleConvert, adds isEmpty() method with Array-specific logic, refactors convert() methods
api/src/org/labkey/api/exp/PropertyType.java	Implements SimpleConvert, updates STRING/MULTI_LINE/XML_TEXT to convert empty strings to null
api/src/org/labkey/api/ontology/Unit.java	Implements SimpleConvert for Quantity conversion
api/src/org/labkey/api/data/ColumnRenderProperties.java	Adds SimpleConvert interface, refactors getConvertFn() to use new pattern
api/src/org/labkey/api/data/BaseColumnInfo.java	Implements getConvertFn() using ColumnRenderProperties.getDefaultConvertFn()
api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java	Implements getConvertFn() delegation
api/src/org/labkey/api/exp/PropertyDescriptor.java	Implements getConvertFn() delegation
api/src/org/labkey/api/data/validator/RequiredValidator.java	Updated to use JdbcType parameter and isEmpty() method for validation
api/src/org/labkey/api/data/validator/ColumnValidators.java	Updated createRequiredValidator to pass JdbcType to RequiredValidator
api/src/org/labkey/api/dataiterator/SimpleTranslator.java	Refactored conversion columns to use ColumnInfo.convert(), removed PropertyConvertColumn classes
api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java	Updated to use simplified addConvertColumn signature
api/src/org/labkey/api/data/ConvertHelper.java	Added getSimpleConvert() factory method, updated DateFriendlyStringConverter to handle empty strings
api/src/org/labkey/api/data/TableViewForm.java	Refactored to use col.convert() instead of ConvertUtils.convert()
api/src/org/labkey/api/data/MultiChoice.java	Updated DisplayColumn to implement IMultiValuedDisplayColumn, improved rendering
api/src/org/labkey/api/query/*.java	Multiple query service classes updated to use col.convert()
experiment/src/org/labkey/experiment/api/*.java	Multiple experiment classes updated to use col.convert() or type.convert()
query/src/org/labkey/query/sql/Method.java	Added ArrayIsEmptyMethod for array_is_empty SQL function
api/src/org/labkey/api/webdav/WebdavResource.java	Added getSpringResource() helper method
api/webapp/WEB-INF/web.xml	Updated to Servlet 3.0 spec, added async-supported to filters
api/src/org/labkey/api/data/ColumnInfoTests.jsp	New test file for ColumnInfo.convert() integration testing
api/src/messages/Validation.properties	Fixed typo: "tyepMismatch" -> "typeMismatch"
core/src/org/labkey/core/CoreModule.java	Added ColumnInfoTests.jsp as integration test

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

api/src/org/labkey/api/webdav/WebdavResource.java

api/src/org/labkey/api/data/ColumnInfoTests.jsp

Copilot · 2026-01-20T20:57:08Z

api/src/org/labkey/api/dataiterator/SimpleTranslator.java

+        }        final String name = col.getName();
+
+



There appears to be orphaned code on line 1347. The variable declaration final String name = col.getName(); is defined but never used in this scope, and appears to be a remnant from refactoring. This dead code should be removed.

Suggested change

} final String name = col.getName();

}

yep should remove

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

Copilot · 2026-01-20T20:57:08Z

api/src/org/labkey/api/webdav/WebdavResource.java

+            @Override
+            public URL getURL() throws IOException
+            {
+                // Nnot sure why getExceuteHref() takes a view context. We ay not need for the moment.


There are spelling errors in the comment. "Nnot" should be "Not", "getExceuteHref" should be "getExecuteHref", and "We ay" should be "We may".

I'm not sure if the current callers handle FIELD_MARKER???.

update DomainKind.hasNullValues() to handle empty arrays, also a faster version.

api/src/org/labkey/api/exp/PropertyType.java

XingY · 2026-01-23T23:33:18Z

api/src/org/labkey/api/exp/property/AbstractDomainKind.java

+            return new SqlSelector(ExperimentService.get().getSchema(), blankRowsSQL).exists();
+        }
+
        if (getTotalAndNonBlankSql(domain, prop, allRowsSQL, nonBlankRowsSQL))


For non-provisioned domain, why do we need a second check if getBlanksSql(domain, prop, blankRowsSQL) is false?

I think there may be a case where the domain supports provisioning (null != getStorageSchemaName()), but no table has been created yet (null == domain.getStorageTableName()).

In that case both methods could return false. I just tried to keep the flow mostly the same.

XingY · 2026-01-23T23:34:25Z

api/src/org/labkey/api/dataiterator/SimpleTranslator.java

+        }        final String name = col.getName();
+
+



yep should remove

XingY · 2026-01-23T23:35:18Z

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

-                        // error
-                        if (col.isMvEnabled() && col.isNullable())
+                    // if the column is mv-enabled and a mv indicator has been specified, don't flag the required error
+                    if (col.isMvEnabled() && col.isNullable())


This should only be checked if requiredError is true, right?

XingY · 2026-01-23T23:37:02Z

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

+        @Override
+        public boolean isEmpty(Object value)
+        {
+            if (null == value)


Should we check for JSONArray to be consistent with other places?

XingY and others added 30 commits January 8, 2026 12:39

Multi choices

685f9ed

Multi value text choices

322bd91

fix empty array

672a750

crlf

11f042c

support filters

65538dc

Merge remote-tracking branch 'origin/develop' into fb_mvtc

5a017c6

clean

9801a9c

fix

afbfda0

order inputs

c628cc8

Merge remote-tracking branch 'origin/develop' into fb_mvtc

3283986

order elements of a MultiChoice.Array

16150fc

explicit null/isBlank() tests

3013578

bug fixes

74f54e4

fix assay import

9e711cc

fix assay import

21cc32b

code review changes

4aabc6b

show advanced settings

defb3c1

Fix study dataset multi value crud

0edb7ec

code review changes

c34b984

Merge remote-tracking branch 'origin/develop' into fb_mvtc

6630ad0

code review changes

a96ac32

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

61693b3

SimpleConvert

3059b18

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvert - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

SimpleConvert

421a026

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvertmake JdbcType.CHAR.convert() consistent with JdbcType.VARCHAR - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

JdbcType.isEmpty()

b446305

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

e3e491b

add to the right branch

e7b3df9

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

b1a9acd

merge from develop

6740d1a

fix selectRows api

03c2f64

XingY requested review from XingY and Copilot January 20, 2026 20:46

Copilot started reviewing on behalf of XingY January 20, 2026 20:47 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

XingY and others added 16 commits January 20, 2026 13:30

add compare type for array is null

b2c1fca

fix check

e4b9f51

fix comparator

910d2a0

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

5212b4e

catch NumberFormatException

fc00dfa

fix assert

b4d0aec

Make adaptParameterMap() work better with TableViewForm.

2f77931

I'm not sure if the current callers handle FIELD_MARKER???.

Converter should handle type=String

3599420

NOT NULL DEFAULT '{}'::text[]

b957788

tests

dc79800

Merge branch 'develop' into fb_mvtc_empty

398243c

null test

7da953f

Don't skip convert

62799dc

isRequiredForInsert ignore getJdbcDefaultValue() for ARRAY type

42775c5

MultiChoice does not need to implement IMultiValuedDisplayColumn

9009f7f

update DomainKind.hasNullValues() to handle empty arrays, also a faster version.

Boolean tests

a229147

XingY reviewed Jan 23, 2026

View reviewed changes

typos

d5ada22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mvtc empty #7348

mvtc empty #7348

Uh oh!

labkey-matthewb commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

XingY Jan 23, 2026

Uh oh!

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Uh oh!

XingY Jan 23, 2026

Uh oh!

labkey-matthewb Jan 23, 2026

Uh oh!

XingY Jan 23, 2026

Uh oh!

XingY Jan 23, 2026

Uh oh!

XingY Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mvtc empty #7348

Are you sure you want to change the base?

mvtc empty #7348

Uh oh!

Conversation

labkey-matthewb commented Jan 20, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

XingY Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

XingY Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

labkey-matthewb Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

XingY Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

XingY Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

XingY Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants