Skip to content

Conversation

@labkey-nicka
Copy link
Contributor

@labkey-nicka labkey-nicka commented Oct 27, 2025

Rationale

This addresses Issue 53567 by refactoring the "Alias" column for data classes and sample types away from the traditional multi-value foreign key (MVFK) to an inline join on the values.

Related Pull Requests

Changes

  • Alias column is now an array of string values.
  • Alias is no longer a lookup column.
  • Introduce ColumnInfo.isMultiValued() to allow columns to declare they are multi-value. Formerly, everything needed to be an instance of MultiValueForeignKey.
  • Refactor group concat sql generation for postgres to use string_agg() and ::text. Reduces per item processing.
  • Consolidate duplicated logic for coercing type values in query update services.
  • Migrate regression test for Issue 43241.

@labkey-nicka labkey-nicka force-pushed the fb_alias_perf_53567 branch 4 times, most recently from cee7b80 to 0f0bb96 Compare November 3, 2025 20:08
addColumn(to, i);
else if (to.getFk() instanceof MultiValuedForeignKey)
else if (to.isMultiValued() || to.getFk() instanceof MultiValuedForeignKey)
addColumn(to.getName(), i); // pass-through multi-value columns -- converting will stringify a collection
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yes ConvertUtils.convert() is bad this way... Maybe, we need to stop using it directly?

@labkey-matthewb labkey-matthewb self-requested a review November 3, 2025 21:11
@labkey-nicka labkey-nicka force-pushed the fb_alias_perf_53567 branch 2 times, most recently from 1caa7e7 to a09092d Compare November 6, 2025 22:17
@labkey-nicka labkey-nicka merged commit e117506 into develop Nov 7, 2025
7 of 14 checks passed
@labkey-nicka labkey-nicka deleted the fb_alias_perf_53567 branch November 7, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants