Skip to content

Conversation

@awln-temporal
Copy link
Contributor

What changed?

Silently ignore unregistered CHASM search attributes for SQL visibility store.
ES already ignores unregistered search attributes (custom or chasm).

Why?

ListWorkflowExecutions needs to succeed when querying on all TemporalNamespaceDivisions.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@awln-temporal awln-temporal requested review from a team as code owners December 8, 2025 17:27
tp, err := combinedTypeMap.GetType(name)
if err != nil {
// Silently ignore ErrInvalidName for unregistered chasm search attributes
if sadefs.IsChasmSearchAttribute(name) && errors.Is(err, searchattribute.ErrInvalidName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The combinedTypeMap already contains the CHASM search attributes. Do you need to check IsChasmSearchAttribute here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, since the chasm search search attributes are not registered when calling ListWorkflowExecutions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, in this case, isn't it better to check before calling combinedTypeMap.GetType?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, since the chasm search attribute could be registered, in which case we do want to try returning it. visibility store implementations are unified for ListWorkflowExecutions and ListChasmExecutions as of now.

@awln-temporal awln-temporal merged commit 5bccff3 into main Dec 12, 2025
58 checks passed
@awln-temporal awln-temporal deleted the chasm-vis-read-ignore-unregistered branch December 12, 2025 00:42
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.

3 participants