Search vector generation does not support fields behind a WorkspaceGate

[eliasylonen]

Description

When a field is gated behind a feature flag using the WorkspaceGate decorator, the columns for the field are correctly not created in the database. However, if the field is included in search field configuration (e.g. SEARCH_FIELDS_FOR_NOTES), database:reset fails when the feature flag is disabled.

Steps to Reproduce

1. Create a feature flag, e.g. IsRichTextV2Enabled

2. In note.workspace-entity.ts, add a feature-gated field using WorkspaceGate decorator:

@WorkspaceField({
  standardId: NOTE_STANDARD_FIELD_IDS.bodyV2,
  type: FieldMetadataType.RICH_TEXT_V2,
  label: 'Body',
  description: 'Note body',
  icon: 'IconFilePencil',
})
@WorkspaceIsNullable()
@WorkspaceGate({
  featureFlag: FeatureFlagKey.IsRichTextV2Enabled,
})
[BODY_V2_FIELD_NAME]: RichTextV2Metadata | null;

3. In the same file, add the new bodyV2 field to the search field configuration:

const TITLE_FIELD_NAME = 'title';
const BODY_FIELD_NAME = 'body';
const BODY_V2_FIELD_NAME = 'bodyV2';

export const SEARCH_FIELDS_FOR_NOTES: FieldTypeAndNameMetadata[] = [
  { name: TITLE_FIELD_NAME, type: FieldMetadataType.TEXT },
  { name: BODY_FIELD_NAME, type: FieldMetadataType.RICH_TEXT },
  { name: BODY_V2_FIELD_NAME, type: FieldMetadataType.RICH_TEXT_V2 },
];

4. Run npx nx run twenty-server:database:reset

An SQL query fails because the database column for the feature gated field doesn't exist:

ALTER TABLE "workspace_1wgvd1injqtife6y4rvfbu3h5"."note" ADD "searchVector" tsvector GENERATED ALWAYS AS (to_tsvector('simple', COALESCE(NULLIF("title"::text, ''), '') || ' ' || COALESCE(NULLIF("body"::text, ''), '') || ' ' || COALESCE(NULLIF("bodyV2Markdown"::text, ''), ''))) STORED

error: column "bodyV2Markdown" does not exist

Possible Solutions

1. Include a missing column check in the generated SQL for fields that have a WorkspaceGate.
2. Something else, maybe bigger changes to searchVector value generation so that the SQL would not include columns behind a WorkspaceGate when the feature flag is disabled for the workspace.

[lucasbordeau]

@Weiko @ijreilly

[ijreilly]

Hi thanks lucas I will take a look !

[ijreilly]

Hi @eliasylonen
I am not surprised by this behavior, as you indicate a potientially non-existent field in the search vector configuration. I am not sure it should be the search vector engine's responsibility to check whether or not the field exists: then the declared configuration would be misleading as it would potentially not represent the reality. (For more context, the search vector is a generated column that brings together the content of other columns - here, title and body columns - to ease search on an Object.)

Instead, I think we should only add the new field to the search vector configuration if it exists:

const SEARCH_FIELDS_FOR_NOTES: FieldTypeAndNameMetadata[] = [ { name: TITLE_FIELD_NAME, type: FieldMetadataType.TEXT }, { name: BODY_FIELD_NAME, type: FieldMetadataType.RICH_TEXT }, ...(isRichTextV2Enabled ? [{ name: BODY_V2_FIELD_NAME, type: FieldMetadataType.RICH_TEXT_V2 }] : []) ];

what do you think?

It also makes me realize we will probably need to run a command to re-generate the search vector fields for the existing metadata when we enable isRichTextV2Enabled as this is not dealt with by sync. I can help with that!

[eliasylonen]

I think we can't access the value of isRichTextV2Enabled when defining SEARCH_FIELDS_FOR_NOTES? The value of the feature flag is different for each workspace. I can't think of an easy way to access isRichTextV2Enabled when defining SEARCH_FIELDS_FOR_NOTES, if there's a way that would probably be the nicest solution.

What about renaming the configuration to something like POSSIBLE_SEARCH_FIELDS_FOR_NOTES and then in the search vector engine adding a column existence check to the generated SQL if there's a WorkspaceGate, do you think that would be too ugly @ijreilly? I think it could be good!

Maybe a quick call with you and @Weiko?

[ijreilly]

@eliasylonen understood, I think maybe we can keep the configuration as it is today for now (we can even remove body if needed from the configuration as it works very poorly for now i think) , and update the configuration (using the new field) when we remove the feature flag. In parallel we will have to run a command to update the search vector for the existing workspaces but that is something we will have to do anyway. what do you think @Weiko ?
Users will still be able to search through notes from their title in the meantime, which is more or less the case today (as said above search on body does not work well)

[ijreilly]

if that's unclear we can have a call yes

[lucasbordeau]

@ijreilly Is it possible to filter the array in getTsVectorColumnExpressionFromFields, and is it the only place where we should do that in the backend ?

[lucasbordeau]

Can we also chose to not add this new field type to the search and remove the search for the old body field also and create a separate issue for this problem ? That would remove a feature but since it seems to not work well right now, maybe that's a good trade-off ?

[eliasylonen]

I think RICH_TEXT_V2 will automatically fix search quality because we can use the markdown column for search instead of blocknote that is JSON.

So I think yes it could be good to remove the existing body search feature.

I think it could be nice to filter the array in getTsVectorColumnExpressionFromFields but I think we can also just wait until RICH_TEXT_V2 has been fully rolled out for notes and tasks (and feature flag has been removed) before we turn on search.